public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG
@ 2023-08-23 11:03 sahil
  2023-10-11  5:25 ` chandni cherukuri
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: sahil @ 2023-08-23 11:03 UTC (permalink / raw)
  To: devel; +Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar, Sahil

NT_FW_CONFIG DTB contains platform information passed by
Tf-A boot stage.
This information is used for Virtual memory map generation
during PEI phase and passed on to DXE phase as a HOB, where
it is used in ConfigurationManagerDxe.

This patch adds a PEI to parse NT_FW_CONFIG and pass it to
other PEI modules(as PPI) and DXE modules(as HOB).

Signed-off-by: sahil <sahil@arm.com>
---

Notes:
    v3:
     - Fixed code review comments
     - Added a PEI to parse nt_fw_config

 Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec                                                 |   8 +-
 Platform/ARM/N1Sdp/N1SdpPlatform.dsc                                                        |   5 +-
 Platform/ARM/N1Sdp/N1SdpPlatform.fdf                                                        |   3 +-
 Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |   6 +-
 Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf                             |  41 ++++++
 Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf                               |   8 +-
 Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h                                           |  14 +--
 Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      |  24 ++--
 Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c                               | 133 ++++++++++++++++++++
 Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c                                 |  12 +-
 Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c                              |  33 +++--
 Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S                              |   4 +-
 12 files changed, 255 insertions(+), 36 deletions(-)

diff --git a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
index d59f25a5b915..7118da25dce0 100644
--- a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
+++ b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
@@ -1,7 +1,7 @@
 ## @file
 #  Describes the entire platform configuration.
 #
-#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
+#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -22,6 +22,8 @@
   Include                        # Root include for the package
 
 [Guids.common]
+  # ARM NeoverseN1Soc Platform Info descriptor
+  gArmNeoverseN1SocPlatformInfoDescriptorGuid = { 0x9fa16eb5, 0xce13, 0x4d37, { 0x96, 0xf0, 0x0a, 0xb5, 0xf1, 0xab, 0xff, 0x01 } }
   gArmNeoverseN1SocTokenSpaceGuid = { 0xab93eb78, 0x60d7, 0x4099, { 0xac, 0xeb, 0x6d, 0xb5, 0x02, 0x58, 0x7c, 0x24 } }
 
 [PcdsFixedAtBuild]
@@ -83,3 +85,7 @@
   gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio32Translation|0x40000000000|UINT64|0x0000004F
   gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation|0x40000000000|UINT64|0x00000050
   gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber|2|UINT32|0x00000051
+
+[Ppis]
+  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid = { 0x21D04AD4, 0x4D23, 0x426D, { 0x8D, 0x3E, 0x92, 0x23, 0x3E, 0xF4, 0xB9, 0x5E } }
+  gArmNeoverseN1SocParameterPpiGuid = { 0x4DDD5A72, 0x31AD, 0x4B20, { 0x8F, 0x5F, 0xB3, 0xE8, 0x24, 0x6F, 0x80, 0x2B } }
diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
index d04b22d3ef51..691d1af3c25c 100644
--- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
+++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
@@ -4,7 +4,7 @@
 # This provides platform specific component descriptions and libraries that
 # conform to EFI/Framework standards.
 #
-# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -228,6 +228,9 @@
   # Platform driver
   Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
 
+  # PEI Phase modules
+  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
+
   # Human Interface Support
   MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
 
diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
index e5e24ea50294..baac8a3ef727 100644
--- a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
+++ b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  FDF file of N1Sdp
 #
-#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
+#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
@@ -195,6 +195,7 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Core/Pei/PeiMain.inf
   INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
   INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+  INF Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
 
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
     SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
index 4f8e7f13021a..242490caf860 100644
--- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
+++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Configuration Manager Dxe
 #
-#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+#  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -42,6 +42,7 @@
 
 [LibraryClasses]
   ArmPlatformLib
+  HobLib
   PrintLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
@@ -170,5 +171,8 @@
   gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation
   gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber
 
+[Guids]
+  gArmNeoverseN1SocPlatformInfoDescriptorGuid
+
 [Depex]
   TRUE
diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
new file mode 100644
index 000000000000..2eec8c327205
--- /dev/null
+++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
@@ -0,0 +1,41 @@
+## @file
+#  This PEI module parse the NtFwConfig for N1Sdp platform and produce
+#  the PPI and HOB.
+#
+#  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = N1SdpNtFwConfigPei
+  FILE_GUID                      = CE76D56C-D3A5-4763-9138-DF09E1D1B614
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = Load
+
+[Sources]
+  NtFwConfigPei.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
+
+[LibraryClasses]
+  DebugLib
+  FdtLib
+  HobLib
+  PeimEntryPoint
+
+[Ppis]
+  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
+  gArmNeoverseN1SocParameterPpiGuid
+
+[Guids]
+  gArmNeoverseN1SocPlatformInfoDescriptorGuid
+
+[Depex]
+  gArmNeoverseN1SocParameterPpiGuid
diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
index 96e590cdd810..6e7e16b86838 100644
--- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
+++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Platform Library for N1Sdp.
 #
-#  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
+#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -58,8 +58,8 @@
 
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
 
-[Guids]
-  gEfiHobListGuid          ## CONSUMES  ## SystemTable
-
 [Ppis]
   gArmMpCoreInfoPpiGuid
+  gArmNeoverseN1SocParameterPpiGuid
+  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
+
diff --git a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
index 097160c7e2d1..2f83d582ccf3 100644
--- a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
+++ b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
@@ -1,6 +1,6 @@
 /** @file
 *
-* Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+* Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
 *
 * SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -41,11 +41,6 @@
 #define NEOVERSEN1SOC_EXP_PERIPH_BASE0               0x1C000000
 #define NEOVERSEN1SOC_EXP_PERIPH_BASE0_SZ            0x1300000
 
-// Base address to a structure of type NEOVERSEN1SOC_PLAT_INFO which is
-// pre-populated by a earlier boot stage
-#define NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE          (NEOVERSEN1SOC_NON_SECURE_SRAM_BASE + \
-                                                      0x00008000)
-
 /*
  * Platform information structure stored in Non-secure SRAM. Platform
  * information are passed from the trusted firmware with the below structure
@@ -55,12 +50,15 @@
 typedef struct {
   /*! 0 - Single Chip, 1 - Chip to Chip (C2C) */
   UINT8   MultichipMode;
-  /*! Slave count in C2C mode */
-  UINT8   SlaveCount;
+  /*! Secondary chip count in C2C mode */
+  UINT8   SecondaryChipCount;
   /*! Local DDR memory size in GigaBytes */
   UINT8   LocalDdrSize;
   /*! Remote DDR memory size in GigaBytes */
   UINT8   RemoteDdrSize;
 } NEOVERSEN1SOC_PLAT_INFO;
 
+typedef struct {
+  CONST VOID    *NtFwConfig;
+} NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI;
 #endif
diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index fa6408a7dd1e..e248826ec925 100644
--- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -1,7 +1,7 @@
 /** @file
   Configuration Manager Dxe
 
-  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -16,6 +16,7 @@
 #include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
 #include <Library/ArmLib.h>
 #include <Library/DebugLib.h>
+#include <Library/HobLib.h>
 #include <Library/IoLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -28,6 +29,7 @@
 #include "Platform.h"
 
 extern struct EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE Hmat;
+static NEOVERSEN1SOC_PLAT_INFO *PlatInfo;
 
 /** The platform configuration repository information.
 */
@@ -1242,13 +1244,21 @@ InitializePlatformRepository (
   IN  EDKII_PLATFORM_REPOSITORY_INFO  * CONST PlatRepoInfo
   )
 {
-  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
   UINT64                        Dram2Size;
   UINT64                        RemoteDdrSize;
+  VOID                          *PlatInfoHob;
+
+  PlatInfoHob = GetFirstGuidHob (&gArmNeoverseN1SocPlatformInfoDescriptorGuid);
+
+  if (PlatInfoHob == NULL) {
+    DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)GET_GUID_HOB_DATA (PlatInfoHob);
 
   RemoteDdrSize = 0;
 
-  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
   Dram2Size = ((PlatInfo->LocalDdrSize - 2) * SIZE_1GB);
 
   PlatRepoInfo->MemAffInfo[LOCAL_DDR_REGION2].Length = Dram2Size;
@@ -1512,7 +1522,6 @@ GetGicCInfo (
   )
 {
   EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
-  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
   UINT32                            TotalObjCount;
   UINT32                            ObjIndex;
 
@@ -1523,7 +1532,6 @@ GetGicCInfo (
   }
 
   PlatformRepo = This->PlatRepoInfo;
-  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
 
   if (PlatInfo->MultichipMode == 1) {
     TotalObjCount = PLAT_CPU_COUNT * 2;
@@ -1623,7 +1631,6 @@ GetStandardNameSpaceObject (
 {
   EFI_STATUS                        Status;
   EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
-  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
   UINT32                            AcpiTableCount;
 
   if ((This == NULL) || (CmObject == NULL)) {
@@ -1634,7 +1641,7 @@ GetStandardNameSpaceObject (
 
   Status = EFI_NOT_FOUND;
   PlatformRepo = This->PlatRepoInfo;
-  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
+
   AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
   if (PlatInfo->MultichipMode == 0)
         AcpiTableCount -= 1;
@@ -1697,7 +1704,6 @@ GetArmNameSpaceObject (
 {
   EFI_STATUS                        Status;
   EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
-  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
   UINT32                            GicRedistCount;
   UINT32                            GicCpuCount;
   UINT32                            ProcHierarchyInfoCount;
@@ -1718,8 +1724,6 @@ GetArmNameSpaceObject (
   Status = EFI_NOT_FOUND;
   PlatformRepo = This->PlatRepoInfo;
 
-  // Probe for multi chip information
-  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
   if (PlatInfo->MultichipMode == 1) {
     GicRedistCount = 2;
     GicCpuCount = PLAT_CPU_COUNT * 2;
diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
new file mode 100644
index 000000000000..ff6a6a7933c3
--- /dev/null
+++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
@@ -0,0 +1,133 @@
+/** @file
+
+  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PeiServicesLib.h>
+
+#include <NeoverseN1Soc.h>
+#include <libfdt.h>
+
+STATIC EFI_PEI_PPI_DESCRIPTOR  gPpi;
+
+/**
+  The entrypoint of the module, parse NtFwConfig and produce the PPI and HOB.
+
+  @param[in]  FileHandle   Handle of the file being invoked.
+  @param[in]  PeiServices  Describes the list of possible PEI Services.
+
+  @retval EFI_SUCCESS      Either no NT_FW_CONFIG was given by EL3 firmware
+                           OR the N1Sdp FDT HOB was successfully created.
+  @retval EFI_UNSUPPORTED  FDT header sanity check failed.
+  @retval *                Other errors are possible.
+**/
+
+EFI_STATUS
+EFIAPI
+Load (
+    IN EFI_PEI_FILE_HANDLE     FileHandle,
+    IN CONST EFI_PEI_SERVICES  **PeiServices
+  )
+{
+  CONST NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI *ParamPpi;
+  CONST UINT32                                 *Property;
+  INT32                                        Offset;
+  NEOVERSEN1SOC_PLAT_INFO                      *PlatInfo;
+  INT32                                        Status;
+
+  PlatInfo = BuildGuidHob (
+               &gArmNeoverseN1SocPlatformInfoDescriptorGuid,
+               sizeof (*PlatInfo)
+               );
+
+  if (PlatInfo == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "[%a]: failed to allocate platform info HOB\n",
+      gEfiCallerBaseName
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = PeiServicesLocatePpi (
+             &gArmNeoverseN1SocParameterPpiGuid,
+             0,
+             NULL,
+             (VOID **)&ParamPpi
+             );
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "[%a]: failed to locate gArmNeoverseN1SocParameterPpiGuid - %r\n",
+      gEfiCallerBaseName,
+      Status
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (fdt_check_header (ParamPpi->NtFwConfig) != 0) {
+    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", ParamPpi->NtFwConfig));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Offset = fdt_subnode_offset (ParamPpi->NtFwConfig, 0, "platform-info");
+  if (Offset == -FDT_ERR_NOTFOUND) {
+    DEBUG ((DEBUG_ERROR, "Invalid DTB : platform-info node not found\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "local-ddr-size", NULL);
+  if (Property == NULL) {
+    DEBUG ((DEBUG_ERROR, "local-ddr-size property not found\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatInfo->LocalDdrSize = fdt32_to_cpu (*Property);
+
+  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "remote-ddr-size", NULL);
+  if (Property == NULL) {
+    DEBUG ((DEBUG_ERROR, "remote-ddr-size property not found\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatInfo->RemoteDdrSize = fdt32_to_cpu (*Property);
+
+  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "secondary-chip-count", NULL);
+  if (Property == NULL) {
+    DEBUG ((DEBUG_ERROR, "secondary-chip-count property not found\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatInfo->SecondaryChipCount = fdt32_to_cpu (*Property);
+
+  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "multichip-mode", NULL);
+  if (Property == NULL) {
+    DEBUG ((DEBUG_ERROR, "multichip-mode property not found\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PlatInfo->MultichipMode = fdt32_to_cpu (*Property);
+
+  gPpi.Flags = EFI_PEI_PPI_DESCRIPTOR_PPI
+               | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
+  gPpi.Guid = &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid;
+  gPpi.Ppi  = PlatInfo;
+
+  Status = PeiServicesInstallPpi (&gPpi);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "[%a]: failed to install PEI service - %r\n",
+      gEfiCallerBaseName,
+      Status
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
index c0effd37f333..52ee6299b3fe 100644
--- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
+++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -8,8 +8,12 @@
 
 #include <Library/ArmPlatformLib.h>
 #include <Library/BaseLib.h>
+#include <NeoverseN1Soc.h>
 #include <Ppi/ArmMpCoreInfo.h>
 
+UINT64  gArgNtFwConfigDtPtr;
+STATIC  NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI mNeoverseN1SocParameterPpi;
+
 STATIC ARM_CORE_INFO mCoreInfoTable[] = {
   { 0x0, 0x0 }, // Cluster 0, Core 0
   { 0x0, 0x1 }, // Cluster 0, Core 1
@@ -46,6 +50,7 @@ ArmPlatformInitialize (
   IN     UINTN                  MpId
   )
 {
+  mNeoverseN1SocParameterPpi.NtFwConfig = (VOID *)gArgNtFwConfigDtPtr;
   return RETURN_SUCCESS;
 }
 
@@ -80,6 +85,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
     EFI_PEI_PPI_DESCRIPTOR_PPI,
     &gArmMpCoreInfoPpiGuid,
     &mMpCoreInfoPpi
+  },
+  {
+    EFI_PEI_PPI_DESCRIPTOR_PPI,
+    &gArmNeoverseN1SocParameterPpiGuid,
+    &mNeoverseN1SocParameterPpi
   }
 };
 
diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
index 9e8a1efc557d..d6ef92677231 100644
--- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
+++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
+  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -10,6 +10,7 @@
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/PeiServicesLib.h>
 #include <NeoverseN1Soc.h>
 
 // The total number of descriptors, including the final "end-of-table" descriptor.
@@ -30,15 +31,31 @@ ArmPlatformGetVirtualMemoryMap (
   IN     ARM_MEMORY_REGION_DESCRIPTOR  **VirtualMemoryMap
   )
 {
-  UINTN                         Index;
-  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
-  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
-  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
-  UINT64                        DramBlock2Size;
-  UINT64                        RemoteDdrSize;
+  UINTN                          Index;
+  ARM_MEMORY_REGION_DESCRIPTOR   *VirtualMemoryTable;
+  EFI_RESOURCE_ATTRIBUTE_TYPE    ResourceAttributes;
+  CONST NEOVERSEN1SOC_PLAT_INFO  *PlatInfo;
+  UINT64                         DramBlock2Size;
+  UINT64                         RemoteDdrSize;
+  EFI_STATUS                     Status;
 
   Index = 0;
-  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
+
+  Status = PeiServicesLocatePpi (
+             &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid,
+             0,
+             NULL,
+             (VOID **)&PlatInfo
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "[%a]: failed to locate gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid - %r\n",
+      gEfiCallerBaseName,
+      Status
+      ));
+  }
+
   DramBlock2Size = ((UINT64)(PlatInfo->LocalDdrSize -
                              NEOVERSEN1SOC_DRAM_BLOCK1_SIZE / SIZE_1GB) *
                             (UINT64)SIZE_1GB);
diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
index 8d2069dea837..cbbe7aae9858 100644
--- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
+++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
+*  Copyright (c) 2019 - 2023, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -25,6 +25,8 @@ GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
 // the UEFI firmware through the CPU registers.
 //
 ASM_PFX(ArmPlatformPeiBootAction):
+  adr  x10, gArgNtFwConfigDtPtr
+  str  x0, [x10]
   ret
 
 //
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107975): https://edk2.groups.io/g/devel/message/107975
Mute This Topic: https://groups.io/mt/100912169/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG
  2023-08-23 11:03 [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG sahil
@ 2023-10-11  5:25 ` chandni cherukuri
  2023-12-07 10:52 ` Thomas Abraham
  2023-12-12 17:10 ` Sami Mujawar
  2 siblings, 0 replies; 4+ messages in thread
From: chandni cherukuri @ 2023-10-11  5:25 UTC (permalink / raw)
  To: devel, sahil; +Cc: Ard Biesheuvel, quic_llindhol, Sami Mujawar

Hi Sahil,

These changes look good to me.

Reviewed-by: Chandni Cherukuri <chandni.cherukuri@arm.com>

Regards,
Chandni Cherukuri

On Wed, Aug 23, 2023 at 4:33 PM sahil <sahil@arm.com> wrote:
>
> NT_FW_CONFIG DTB contains platform information passed by
> Tf-A boot stage.
> This information is used for Virtual memory map generation
> during PEI phase and passed on to DXE phase as a HOB, where
> it is used in ConfigurationManagerDxe.
>
> This patch adds a PEI to parse NT_FW_CONFIG and pass it to
> other PEI modules(as PPI) and DXE modules(as HOB).
>
> Signed-off-by: sahil <sahil@arm.com>
> ---
>
> Notes:
>     v3:
>      - Fixed code review comments
>      - Added a PEI to parse nt_fw_config
>
>  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec                                                 |   8 +-
>  Platform/ARM/N1Sdp/N1SdpPlatform.dsc                                                        |   5 +-
>  Platform/ARM/N1Sdp/N1SdpPlatform.fdf                                                        |   3 +-
>  Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |   6 +-
>  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf                             |  41 ++++++
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf                               |   8 +-
>  Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h                                           |  14 +--
>  Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      |  24 ++--
>  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c                               | 133 ++++++++++++++++++++
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c                                 |  12 +-
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c                              |  33 +++--
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S                              |   4 +-
>  12 files changed, 255 insertions(+), 36 deletions(-)
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> index d59f25a5b915..7118da25dce0 100644
> --- a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> +++ b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Describes the entire platform configuration.
>  #
> -#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -22,6 +22,8 @@
>    Include                        # Root include for the package
>
>  [Guids.common]
> +  # ARM NeoverseN1Soc Platform Info descriptor
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid = { 0x9fa16eb5, 0xce13, 0x4d37, { 0x96, 0xf0, 0x0a, 0xb5, 0xf1, 0xab, 0xff, 0x01 } }
>    gArmNeoverseN1SocTokenSpaceGuid = { 0xab93eb78, 0x60d7, 0x4099, { 0xac, 0xeb, 0x6d, 0xb5, 0x02, 0x58, 0x7c, 0x24 } }
>
>  [PcdsFixedAtBuild]
> @@ -83,3 +85,7 @@
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio32Translation|0x40000000000|UINT64|0x0000004F
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation|0x40000000000|UINT64|0x00000050
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber|2|UINT32|0x00000051
> +
> +[Ppis]
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid = { 0x21D04AD4, 0x4D23, 0x426D, { 0x8D, 0x3E, 0x92, 0x23, 0x3E, 0xF4, 0xB9, 0x5E } }
> +  gArmNeoverseN1SocParameterPpiGuid = { 0x4DDD5A72, 0x31AD, 0x4B20, { 0x8F, 0x5F, 0xB3, 0xE8, 0x24, 0x6F, 0x80, 0x2B } }
> diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> index d04b22d3ef51..691d1af3c25c 100644
> --- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> @@ -4,7 +4,7 @@
>  # This provides platform specific component descriptions and libraries that
>  # conform to EFI/Framework standards.
>  #
> -# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
> +# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -228,6 +228,9 @@
>    # Platform driver
>    Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
>
> +  # PEI Phase modules
> +  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> +
>    # Human Interface Support
>    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>
> diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> index e5e24ea50294..baac8a3ef727 100644
> --- a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  FDF file of N1Sdp
>  #
> -#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  ##
> @@ -195,6 +195,7 @@ READ_LOCK_STATUS   = TRUE
>    INF MdeModulePkg/Core/Pei/PeiMain.inf
>    INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>    INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +  INF Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
>
>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> index 4f8e7f13021a..242490caf860 100644
> --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Configuration Manager Dxe
>  #
> -#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +#  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -42,6 +42,7 @@
>
>  [LibraryClasses]
>    ArmPlatformLib
> +  HobLib
>    PrintLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> @@ -170,5 +171,8 @@
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber
>
> +[Guids]
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid
> +
>  [Depex]
>    TRUE
> diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> new file mode 100644
> index 000000000000..2eec8c327205
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> @@ -0,0 +1,41 @@
> +## @file
> +#  This PEI module parse the NtFwConfig for N1Sdp platform and produce
> +#  the PPI and HOB.
> +#
> +#  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = N1SdpNtFwConfigPei
> +  FILE_GUID                      = CE76D56C-D3A5-4763-9138-DF09E1D1B614
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = Load
> +
> +[Sources]
> +  NtFwConfigPei.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  FdtLib
> +  HobLib
> +  PeimEntryPoint
> +
> +[Ppis]
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
> +  gArmNeoverseN1SocParameterPpiGuid
> +
> +[Guids]
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid
> +
> +[Depex]
> +  gArmNeoverseN1SocParameterPpiGuid
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> index 96e590cdd810..6e7e16b86838 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Platform Library for N1Sdp.
>  #
> -#  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -58,8 +58,8 @@
>
>    gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
> -[Guids]
> -  gEfiHobListGuid          ## CONSUMES  ## SystemTable
> -
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
> +  gArmNeoverseN1SocParameterPpiGuid
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
> +
> diff --git a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> index 097160c7e2d1..2f83d582ccf3 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> +++ b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> @@ -1,6 +1,6 @@
>  /** @file
>  *
> -* Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +* Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
>  *
>  * SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -41,11 +41,6 @@
>  #define NEOVERSEN1SOC_EXP_PERIPH_BASE0               0x1C000000
>  #define NEOVERSEN1SOC_EXP_PERIPH_BASE0_SZ            0x1300000
>
> -// Base address to a structure of type NEOVERSEN1SOC_PLAT_INFO which is
> -// pre-populated by a earlier boot stage
> -#define NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE          (NEOVERSEN1SOC_NON_SECURE_SRAM_BASE + \
> -                                                      0x00008000)
> -
>  /*
>   * Platform information structure stored in Non-secure SRAM. Platform
>   * information are passed from the trusted firmware with the below structure
> @@ -55,12 +50,15 @@
>  typedef struct {
>    /*! 0 - Single Chip, 1 - Chip to Chip (C2C) */
>    UINT8   MultichipMode;
> -  /*! Slave count in C2C mode */
> -  UINT8   SlaveCount;
> +  /*! Secondary chip count in C2C mode */
> +  UINT8   SecondaryChipCount;
>    /*! Local DDR memory size in GigaBytes */
>    UINT8   LocalDdrSize;
>    /*! Remote DDR memory size in GigaBytes */
>    UINT8   RemoteDdrSize;
>  } NEOVERSEN1SOC_PLAT_INFO;
>
> +typedef struct {
> +  CONST VOID    *NtFwConfig;
> +} NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI;
>  #endif
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index fa6408a7dd1e..e248826ec925 100644
> --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Configuration Manager Dxe
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -16,6 +16,7 @@
>  #include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
>  #include <Library/ArmLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> @@ -28,6 +29,7 @@
>  #include "Platform.h"
>
>  extern struct EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE Hmat;
> +static NEOVERSEN1SOC_PLAT_INFO *PlatInfo;
>
>  /** The platform configuration repository information.
>  */
> @@ -1242,13 +1244,21 @@ InitializePlatformRepository (
>    IN  EDKII_PLATFORM_REPOSITORY_INFO  * CONST PlatRepoInfo
>    )
>  {
> -  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
>    UINT64                        Dram2Size;
>    UINT64                        RemoteDdrSize;
> +  VOID                          *PlatInfoHob;
> +
> +  PlatInfoHob = GetFirstGuidHob (&gArmNeoverseN1SocPlatformInfoDescriptorGuid);
> +
> +  if (PlatInfoHob == NULL) {
> +    DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)GET_GUID_HOB_DATA (PlatInfoHob);
>
>    RemoteDdrSize = 0;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>    Dram2Size = ((PlatInfo->LocalDdrSize - 2) * SIZE_1GB);
>
>    PlatRepoInfo->MemAffInfo[LOCAL_DDR_REGION2].Length = Dram2Size;
> @@ -1512,7 +1522,6 @@ GetGicCInfo (
>    )
>  {
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>    UINT32                            TotalObjCount;
>    UINT32                            ObjIndex;
>
> @@ -1523,7 +1532,6 @@ GetGicCInfo (
>    }
>
>    PlatformRepo = This->PlatRepoInfo;
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>    if (PlatInfo->MultichipMode == 1) {
>      TotalObjCount = PLAT_CPU_COUNT * 2;
> @@ -1623,7 +1631,6 @@ GetStandardNameSpaceObject (
>  {
>    EFI_STATUS                        Status;
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>    UINT32                            AcpiTableCount;
>
>    if ((This == NULL) || (CmObject == NULL)) {
> @@ -1634,7 +1641,7 @@ GetStandardNameSpaceObject (
>
>    Status = EFI_NOT_FOUND;
>    PlatformRepo = This->PlatRepoInfo;
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
> +
>    AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
>    if (PlatInfo->MultichipMode == 0)
>          AcpiTableCount -= 1;
> @@ -1697,7 +1704,6 @@ GetArmNameSpaceObject (
>  {
>    EFI_STATUS                        Status;
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>    UINT32                            GicRedistCount;
>    UINT32                            GicCpuCount;
>    UINT32                            ProcHierarchyInfoCount;
> @@ -1718,8 +1724,6 @@ GetArmNameSpaceObject (
>    Status = EFI_NOT_FOUND;
>    PlatformRepo = This->PlatRepoInfo;
>
> -  // Probe for multi chip information
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>    if (PlatInfo->MultichipMode == 1) {
>      GicRedistCount = 2;
>      GicCpuCount = PLAT_CPU_COUNT * 2;
> diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> new file mode 100644
> index 000000000000..ff6a6a7933c3
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> @@ -0,0 +1,133 @@
> +/** @file
> +
> +  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PeiServicesLib.h>
> +
> +#include <NeoverseN1Soc.h>
> +#include <libfdt.h>
> +
> +STATIC EFI_PEI_PPI_DESCRIPTOR  gPpi;
> +
> +/**
> +  The entrypoint of the module, parse NtFwConfig and produce the PPI and HOB.
> +
> +  @param[in]  FileHandle   Handle of the file being invoked.
> +  @param[in]  PeiServices  Describes the list of possible PEI Services.
> +
> +  @retval EFI_SUCCESS      Either no NT_FW_CONFIG was given by EL3 firmware
> +                           OR the N1Sdp FDT HOB was successfully created.
> +  @retval EFI_UNSUPPORTED  FDT header sanity check failed.
> +  @retval *                Other errors are possible.
> +**/
> +
> +EFI_STATUS
> +EFIAPI
> +Load (
> +    IN EFI_PEI_FILE_HANDLE     FileHandle,
> +    IN CONST EFI_PEI_SERVICES  **PeiServices
> +  )
> +{
> +  CONST NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI *ParamPpi;
> +  CONST UINT32                                 *Property;
> +  INT32                                        Offset;
> +  NEOVERSEN1SOC_PLAT_INFO                      *PlatInfo;
> +  INT32                                        Status;
> +
> +  PlatInfo = BuildGuidHob (
> +               &gArmNeoverseN1SocPlatformInfoDescriptorGuid,
> +               sizeof (*PlatInfo)
> +               );
> +
> +  if (PlatInfo == NULL) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "[%a]: failed to allocate platform info HOB\n",
> +      gEfiCallerBaseName
> +      ));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = PeiServicesLocatePpi (
> +             &gArmNeoverseN1SocParameterPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **)&ParamPpi
> +             );
> +
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "[%a]: failed to locate gArmNeoverseN1SocParameterPpiGuid - %r\n",
> +      gEfiCallerBaseName,
> +      Status
> +      ));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (fdt_check_header (ParamPpi->NtFwConfig) != 0) {
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", ParamPpi->NtFwConfig));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Offset = fdt_subnode_offset (ParamPpi->NtFwConfig, 0, "platform-info");
> +  if (Offset == -FDT_ERR_NOTFOUND) {
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB : platform-info node not found\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "local-ddr-size", NULL);
> +  if (Property == NULL) {
> +    DEBUG ((DEBUG_ERROR, "local-ddr-size property not found\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  PlatInfo->LocalDdrSize = fdt32_to_cpu (*Property);
> +
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "remote-ddr-size", NULL);
> +  if (Property == NULL) {
> +    DEBUG ((DEBUG_ERROR, "remote-ddr-size property not found\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  PlatInfo->RemoteDdrSize = fdt32_to_cpu (*Property);
> +
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "secondary-chip-count", NULL);
> +  if (Property == NULL) {
> +    DEBUG ((DEBUG_ERROR, "secondary-chip-count property not found\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  PlatInfo->SecondaryChipCount = fdt32_to_cpu (*Property);
> +
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "multichip-mode", NULL);
> +  if (Property == NULL) {
> +    DEBUG ((DEBUG_ERROR, "multichip-mode property not found\n"));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  PlatInfo->MultichipMode = fdt32_to_cpu (*Property);
> +
> +  gPpi.Flags = EFI_PEI_PPI_DESCRIPTOR_PPI
> +               | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> +  gPpi.Guid = &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid;
> +  gPpi.Ppi  = PlatInfo;
> +
> +  Status = PeiServicesInstallPpi (&gPpi);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "[%a]: failed to install PEI service - %r\n",
> +      gEfiCallerBaseName,
> +      Status
> +      ));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> index c0effd37f333..52ee6299b3fe 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> @@ -1,6 +1,6 @@
>  /** @file
>
> -  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -8,8 +8,12 @@
>
>  #include <Library/ArmPlatformLib.h>
>  #include <Library/BaseLib.h>
> +#include <NeoverseN1Soc.h>
>  #include <Ppi/ArmMpCoreInfo.h>
>
> +UINT64  gArgNtFwConfigDtPtr;
> +STATIC  NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI mNeoverseN1SocParameterPpi;
> +
>  STATIC ARM_CORE_INFO mCoreInfoTable[] = {
>    { 0x0, 0x0 }, // Cluster 0, Core 0
>    { 0x0, 0x1 }, // Cluster 0, Core 1
> @@ -46,6 +50,7 @@ ArmPlatformInitialize (
>    IN     UINTN                  MpId
>    )
>  {
> +  mNeoverseN1SocParameterPpi.NtFwConfig = (VOID *)gArgNtFwConfigDtPtr;
>    return RETURN_SUCCESS;
>  }
>
> @@ -80,6 +85,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
>      EFI_PEI_PPI_DESCRIPTOR_PPI,
>      &gArmMpCoreInfoPpiGuid,
>      &mMpCoreInfoPpi
> +  },
> +  {
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
> +    &gArmNeoverseN1SocParameterPpiGuid,
> +    &mNeoverseN1SocParameterPpi
>    }
>  };
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> index 9e8a1efc557d..d6ef92677231 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> @@ -1,6 +1,6 @@
>  /** @file
>
> -  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -10,6 +10,7 @@
>  #include <Library/DebugLib.h>
>  #include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>
> +#include <Library/PeiServicesLib.h>
>  #include <NeoverseN1Soc.h>
>
>  // The total number of descriptors, including the final "end-of-table" descriptor.
> @@ -30,15 +31,31 @@ ArmPlatformGetVirtualMemoryMap (
>    IN     ARM_MEMORY_REGION_DESCRIPTOR  **VirtualMemoryMap
>    )
>  {
> -  UINTN                         Index;
> -  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> -  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
> -  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
> -  UINT64                        DramBlock2Size;
> -  UINT64                        RemoteDdrSize;
> +  UINTN                          Index;
> +  ARM_MEMORY_REGION_DESCRIPTOR   *VirtualMemoryTable;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE    ResourceAttributes;
> +  CONST NEOVERSEN1SOC_PLAT_INFO  *PlatInfo;
> +  UINT64                         DramBlock2Size;
> +  UINT64                         RemoteDdrSize;
> +  EFI_STATUS                     Status;
>
>    Index = 0;
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
> +
> +  Status = PeiServicesLocatePpi (
> +             &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **)&PlatInfo
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "[%a]: failed to locate gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid - %r\n",
> +      gEfiCallerBaseName,
> +      Status
> +      ));
> +  }
> +
>    DramBlock2Size = ((UINT64)(PlatInfo->LocalDdrSize -
>                               NEOVERSEN1SOC_DRAM_BLOCK1_SIZE / SIZE_1GB) *
>                              (UINT64)SIZE_1GB);
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> index 8d2069dea837..cbbe7aae9858 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> @@ -1,6 +1,6 @@
>  /** @file
>  *
> -*  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> +*  Copyright (c) 2019 - 2023, ARM Limited. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -25,6 +25,8 @@ GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
>  // the UEFI firmware through the CPU registers.
>  //
>  ASM_PFX(ArmPlatformPeiBootAction):
> +  adr  x10, gArgNtFwConfigDtPtr
> +  str  x0, [x10]
>    ret
>
>  //
> --
> 2.25.1
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#107975): https://edk2.groups.io/g/devel/message/107975
> Mute This Topic: https://groups.io/mt/100912169/5878107
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ch.chandni@gmail.com]
> ------------
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109516): https://edk2.groups.io/g/devel/message/109516
Mute This Topic: https://groups.io/mt/100912169/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG
  2023-08-23 11:03 [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG sahil
  2023-10-11  5:25 ` chandni cherukuri
@ 2023-12-07 10:52 ` Thomas Abraham
  2023-12-12 17:10 ` Sami Mujawar
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Abraham @ 2023-12-07 10:52 UTC (permalink / raw)
  To: devel@edk2.groups.io, Sahil Kaushal
  Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar

Hi Sahil,

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of sahil via
> groups.io
> Sent: Wednesday, August 23, 2023 12:03 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm
> <leif@nuviainc.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Sahil
> <Sahil@arm.com>
> Subject: [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp:
> Add support to parse NT_FW_CONFIG
>
> NT_FW_CONFIG DTB contains platform information passed by
> Tf-A boot stage.
> This information is used for Virtual memory map generation
> during PEI phase and passed on to DXE phase as a HOB, where
> it is used in ConfigurationManagerDxe.
>
> This patch adds a PEI to parse NT_FW_CONFIG and pass it to
> other PEI modules(as PPI) and DXE modules(as HOB).
>
> Signed-off-by: sahil <sahil@arm.com>
> ---
>
> Notes:
>     v3:
>      - Fixed code review comments
>      - Added a PEI to parse nt_fw_config
>
>  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> |   8 +-
>  Platform/ARM/N1Sdp/N1SdpPlatform.dsc                                                        |
> 5 +-
>  Platform/ARM/N1Sdp/N1SdpPlatform.fdf                                                        |
> 3 +-
>
> Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Config
> urationManagerDxe.inf |   6 +-
>  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> |  41 ++++++
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> |   8 +-
>  Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> |  14 +--
>
> Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Config
> urationManager.c      |  24 ++--
>  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> | 133 ++++++++++++++++++++
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> |  12 +-
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> |  33 +++--
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> |   4 +-
>  12 files changed, 255 insertions(+), 36 deletions(-)
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> index d59f25a5b915..7118da25dce0 100644
> --- a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> +++ b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> @@ -1,7 +1,7 @@
>  ## @file
>
>  #  Describes the entire platform configuration.
>
>  #
>
> -#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>  #
>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #
>
> @@ -22,6 +22,8 @@
>    Include                        # Root include for the package
>
>
>
>  [Guids.common]
>
> +  # ARM NeoverseN1Soc Platform Info descriptor
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid = { 0x9fa16eb5, 0xce13,
> 0x4d37, { 0x96, 0xf0, 0x0a, 0xb5, 0xf1, 0xab, 0xff, 0x01 } }
>
>    gArmNeoverseN1SocTokenSpaceGuid = { 0xab93eb78, 0x60d7, 0x4099, { 0xac,
> 0xeb, 0x6d, 0xb5, 0x02, 0x58, 0x7c, 0x24 } }
>
>
>
>  [PcdsFixedAtBuild]
>
> @@ -83,3 +85,7 @@
>
> gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio32Translation|0x400
> 00000000|UINT64|0x0000004F
>
>
> gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation|0x400
> 00000000|UINT64|0x00000050
>
>
> gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber|2|UINT3
> 2|0x00000051
>
> +
>
> +[Ppis]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid = { 0x21D04AD4,
> 0x4D23, 0x426D, { 0x8D, 0x3E, 0x92, 0x23, 0x3E, 0xF4, 0xB9, 0x5E } }
>
> +  gArmNeoverseN1SocParameterPpiGuid = { 0x4DDD5A72, 0x31AD, 0x4B20, {
> 0x8F, 0x5F, 0xB3, 0xE8, 0x24, 0x6F, 0x80, 0x2B } }
>
> diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> index d04b22d3ef51..691d1af3c25c 100644
> --- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> @@ -4,7 +4,7 @@
>  # This provides platform specific component descriptions and libraries that
>
>  # conform to EFI/Framework standards.
>
>  #
>
> -# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>  #
>
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #
>
> @@ -228,6 +228,9 @@
>    # Platform driver
>
>    Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
>
>
>
> +  # PEI Phase modules
>
> +  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
>
> +
>
>    # Human Interface Support
>
>    MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>
>
>
> diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> index e5e24ea50294..baac8a3ef727 100644
> --- a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>
>  #  FDF file of N1Sdp
>
>  #
>
> -#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>  #
>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  ##
>
> @@ -195,6 +195,7 @@ READ_LOCK_STATUS   = TRUE
>    INF MdeModulePkg/Core/Pei/PeiMain.inf
>
>    INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>
>    INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>
> +  INF Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
>
>
>
>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>
>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF
> PROCESSING_REQUIRED = TRUE {
>
> diff --git
> a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManagerDxe.inf
> b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManagerDxe.inf
> index 4f8e7f13021a..242490caf860 100644
> ---
> a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManagerDxe.inf
> +++
> b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManagerDxe.inf
> @@ -1,7 +1,7 @@
>  ## @file
>
>  #  Configuration Manager Dxe
>
>  #
>
> -#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
>
>  #
>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #
>
> @@ -42,6 +42,7 @@
>
>
>  [LibraryClasses]
>
>    ArmPlatformLib
>
> +  HobLib
>
>    PrintLib
>
>    UefiBootServicesTableLib
>
>    UefiDriverEntryPoint
>
> @@ -170,5 +171,8 @@
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation
>
>    gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber
>
>
>
> +[Guids]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid
>
> +
>
>  [Depex]
>
>    TRUE
>
> diff --git
> a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> new file mode 100644
> index 000000000000..2eec8c327205
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> @@ -0,0 +1,41 @@
> +## @file
>
> +#  This PEI module parse the NtFwConfig for N1Sdp platform and produce
>
> +#  the PPI and HOB.
>
> +#
>
> +#  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
>
> +#
>
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +#
>
> +##
>
> +
>
> +[Defines]
>
> +  INF_VERSION                    = 0x0001001B
>
> +  BASE_NAME                      = N1SdpNtFwConfigPei
>
> +  FILE_GUID                      = CE76D56C-D3A5-4763-9138-DF09E1D1B614
>
> +  MODULE_TYPE                    = PEIM
>
> +  VERSION_STRING                 = 1.0
>
> +  ENTRY_POINT                    = Load
>
> +
>
> +[Sources]
>
> +  NtFwConfigPei.c
>
> +
>
> +[Packages]
>
> +  EmbeddedPkg/EmbeddedPkg.dec
>
> +  MdePkg/MdePkg.dec
>
> +  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
>
> +
>
> +[LibraryClasses]
>
> +  DebugLib
>
> +  FdtLib
>
> +  HobLib
>
> +  PeimEntryPoint
>
> +
>
> +[Ppis]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
>
> +  gArmNeoverseN1SocParameterPpiGuid
>
> +
>
> +[Guids]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid
>
> +
>
> +[Depex]
>
> +  gArmNeoverseN1SocParameterPpiGuid
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> index 96e590cdd810..6e7e16b86838 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>
>  #  Platform Library for N1Sdp.
>
>  #
>
> -#  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>  #
>
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #
>
> @@ -58,8 +58,8 @@
>
>
>    gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
>
>
> -[Guids]
>
> -  gEfiHobListGuid          ## CONSUMES  ## SystemTable
>
> -
>
>  [Ppis]
>
>    gArmMpCoreInfoPpiGuid
>
> +  gArmNeoverseN1SocParameterPpiGuid
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
>
> +
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> index 097160c7e2d1..2f83d582ccf3 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> +++ b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> @@ -1,6 +1,6 @@
>  /** @file
>
>  *
>
> -* Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
>
> +* Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
>
>  *
>
>  * SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  *
>
> @@ -41,11 +41,6 @@
>  #define NEOVERSEN1SOC_EXP_PERIPH_BASE0               0x1C000000
>
>  #define NEOVERSEN1SOC_EXP_PERIPH_BASE0_SZ            0x1300000
>
>
>
> -// Base address to a structure of type NEOVERSEN1SOC_PLAT_INFO which is
>
> -// pre-populated by a earlier boot stage
>
> -#define NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE
> (NEOVERSEN1SOC_NON_SECURE_SRAM_BASE + \
>
> -                                                      0x00008000)
>
> -
>
>  /*
>
>   * Platform information structure stored in Non-secure SRAM. Platform
>
>   * information are passed from the trusted firmware with the below structure
>
> @@ -55,12 +50,15 @@
>  typedef struct {
>
>    /*! 0 - Single Chip, 1 - Chip to Chip (C2C) */
>
>    UINT8   MultichipMode;
>
> -  /*! Slave count in C2C mode */
>
> -  UINT8   SlaveCount;
>
> +  /*! Secondary chip count in C2C mode */
>
> +  UINT8   SecondaryChipCount;
>
>    /*! Local DDR memory size in GigaBytes */
>
>    UINT8   LocalDdrSize;
>
>    /*! Remote DDR memory size in GigaBytes */
>
>    UINT8   RemoteDdrSize;
>
>  } NEOVERSEN1SOC_PLAT_INFO;
>
>
>
> +typedef struct {
>
> +  CONST VOID    *NtFwConfig;
>
> +} NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI;
>
>  #endif
>
> diff --git
> a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManager.c
> b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManager.c
> index fa6408a7dd1e..e248826ec925 100644
> ---
> a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManager.c
> +++
> b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/Con
> figurationManager.c
> @@ -1,7 +1,7 @@
>  /** @file
>
>    Configuration Manager Dxe
>
>
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
>
> +  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
>
>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> @@ -16,6 +16,7 @@
>  #include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
>
>  #include <Library/ArmLib.h>
>
>  #include <Library/DebugLib.h>
>
> +#include <Library/HobLib.h>
>
>  #include <Library/IoLib.h>
>
>  #include <Library/PcdLib.h>
>
>  #include <Library/UefiBootServicesTableLib.h>
>
> @@ -28,6 +29,7 @@
>  #include "Platform.h"
>
>
>
>  extern struct EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE Hmat;
>
> +static NEOVERSEN1SOC_PLAT_INFO *PlatInfo;
>
>
>
>  /** The platform configuration repository information.
>
>  */
>
> @@ -1242,13 +1244,21 @@ InitializePlatformRepository (
>    IN  EDKII_PLATFORM_REPOSITORY_INFO  * CONST PlatRepoInfo
>
>    )
>
>  {
>
> -  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
>
>    UINT64                        Dram2Size;
>
>    UINT64                        RemoteDdrSize;
>
> +  VOID                          *PlatInfoHob;
>
> +
>
> +  PlatInfoHob = GetFirstGuidHob
> (&gArmNeoverseN1SocPlatformInfoDescriptorGuid);
>
> +
>
> +  if (PlatInfoHob == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
>
> +    return EFI_NOT_FOUND;
>
> +  }
>
> +
>
> +  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)GET_GUID_HOB_DATA
> (PlatInfoHob);
>
>
>
>    RemoteDdrSize = 0;
>
>
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO
> *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>    Dram2Size = ((PlatInfo->LocalDdrSize - 2) * SIZE_1GB);
>
>
>
>    PlatRepoInfo->MemAffInfo[LOCAL_DDR_REGION2].Length = Dram2Size;
>
> @@ -1512,7 +1522,6 @@ GetGicCInfo (
>    )
>
>  {
>
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
>
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>
>    UINT32                            TotalObjCount;
>
>    UINT32                            ObjIndex;
>
>
>
> @@ -1523,7 +1532,6 @@ GetGicCInfo (
>    }
>
>
>
>    PlatformRepo = This->PlatRepoInfo;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO
> *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>
>
>    if (PlatInfo->MultichipMode == 1) {
>
>      TotalObjCount = PLAT_CPU_COUNT * 2;
>
> @@ -1623,7 +1631,6 @@ GetStandardNameSpaceObject (
>  {
>
>    EFI_STATUS                        Status;
>
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
>
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>
>    UINT32                            AcpiTableCount;
>
>
>
>    if ((This == NULL) || (CmObject == NULL)) {
>
> @@ -1634,7 +1641,7 @@ GetStandardNameSpaceObject (
>
>
>    Status = EFI_NOT_FOUND;
>
>    PlatformRepo = This->PlatRepoInfo;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO
> *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
> +
>
>    AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
>
>    if (PlatInfo->MultichipMode == 0)
>
>          AcpiTableCount -= 1;
>
> @@ -1697,7 +1704,6 @@ GetArmNameSpaceObject (
>  {
>
>    EFI_STATUS                        Status;
>
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
>
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>
>    UINT32                            GicRedistCount;
>
>    UINT32                            GicCpuCount;
>
>    UINT32                            ProcHierarchyInfoCount;
>
> @@ -1718,8 +1724,6 @@ GetArmNameSpaceObject (
>    Status = EFI_NOT_FOUND;
>
>    PlatformRepo = This->PlatRepoInfo;
>
>
>
> -  // Probe for multi chip information
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO
> *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>    if (PlatInfo->MultichipMode == 1) {
>
>      GicRedistCount = 2;
>
>      GicCpuCount = PLAT_CPU_COUNT * 2;
>
> diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> new file mode 100644
> index 000000000000..ff6a6a7933c3
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> @@ -0,0 +1,133 @@
> +/** @file
>
> +
>
> +  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#include <Library/DebugLib.h>
>
> +#include <Library/HobLib.h>
>
> +#include <Library/PeiServicesLib.h>
>
> +
>
> +#include <NeoverseN1Soc.h>
>
> +#include <libfdt.h>
>
> +
>
> +STATIC EFI_PEI_PPI_DESCRIPTOR  gPpi;
>
> +
>
> +/**
>
> +  The entrypoint of the module, parse NtFwConfig and produce the PPI and
> HOB.
>
> +
>
> +  @param[in]  FileHandle   Handle of the file being invoked.
>
> +  @param[in]  PeiServices  Describes the list of possible PEI Services.
>
> +
>
> +  @retval EFI_SUCCESS      Either no NT_FW_CONFIG was given by EL3
> firmware
>
> +                           OR the N1Sdp FDT HOB was successfully created.
>
> +  @retval EFI_UNSUPPORTED  FDT header sanity check failed.
>
> +  @retval *                Other errors are possible.
>
> +**/
>
> +
>
> +EFI_STATUS
>
> +EFIAPI
>
> +Load (

This could have been named better.

>
> +    IN EFI_PEI_FILE_HANDLE     FileHandle,
>
> +    IN CONST EFI_PEI_SERVICES  **PeiServices
>
> +  )
>
> +{
>
> +  CONST NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI *ParamPpi;
>
> +  CONST UINT32                                 *Property;
>
> +  INT32                                        Offset;
>
> +  NEOVERSEN1SOC_PLAT_INFO                      *PlatInfo;
>
> +  INT32                                        Status;
>
> +
>
> +  PlatInfo = BuildGuidHob (
>
> +               &gArmNeoverseN1SocPlatformInfoDescriptorGuid,
>
> +               sizeof (*PlatInfo)
>
> +               );
>
> +
>
> +  if (PlatInfo == NULL) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to allocate platform info HOB\n",
>
> +      gEfiCallerBaseName
>
> +      ));
>
> +    return EFI_INVALID_PARAMETER;

Is this the correct return value?

>
> +  }
>
> +
>
> +  Status = PeiServicesLocatePpi (
>
> +             &gArmNeoverseN1SocParameterPpiGuid,
>
> +             0,
>
> +             NULL,
>
> +             (VOID **)&ParamPpi
>
> +             );
>
> +
>
> +  if (EFI_ERROR (Status)) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to locate gArmNeoverseN1SocParameterPpiGuid - %r\n",
>
> +      gEfiCallerBaseName,
>
> +      Status
>
> +      ));
>
> +    return EFI_INVALID_PARAMETER;

Same here and the below return values as well.

>
> +  }
>
> +
>
> +  if (fdt_check_header (ParamPpi->NtFwConfig) != 0) {
>
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", ParamPpi-
> >NtFwConfig));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  Offset = fdt_subnode_offset (ParamPpi->NtFwConfig, 0, "platform-info");
>
> +  if (Offset == -FDT_ERR_NOTFOUND) {
>
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB : platform-info node not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "local-ddr-size",
> NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "local-ddr-size property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->LocalDdrSize = fdt32_to_cpu (*Property);
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "remote-ddr-size",
> NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "remote-ddr-size property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->RemoteDdrSize = fdt32_to_cpu (*Property);
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "secondary-chip-
> count", NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "secondary-chip-count property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->SecondaryChipCount = fdt32_to_cpu (*Property);
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "multichip-mode",
> NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "multichip-mode property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->MultichipMode = fdt32_to_cpu (*Property);
>
> +
>
> +  gPpi.Flags = EFI_PEI_PPI_DESCRIPTOR_PPI
>
> +               | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
>
> +  gPpi.Guid = &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid;
>
> +  gPpi.Ppi  = PlatInfo;
>
> +
>
> +  Status = PeiServicesInstallPpi (&gPpi);
>
> +  if (EFI_ERROR (Status)) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to install PEI service - %r\n",
>
> +      gEfiCallerBaseName,
>
> +      Status
>
> +      ));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  return EFI_SUCCESS;
>
> +}
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> index c0effd37f333..52ee6299b3fe 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> @@ -1,6 +1,6 @@
>  /** @file
>
>
>
> -  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
>
> +  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> @@ -8,8 +8,12 @@
>
>
>  #include <Library/ArmPlatformLib.h>
>
>  #include <Library/BaseLib.h>
>
> +#include <NeoverseN1Soc.h>
>
>  #include <Ppi/ArmMpCoreInfo.h>
>
>
>
> +UINT64  gArgNtFwConfigDtPtr;
>
> +STATIC  NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI
> mNeoverseN1SocParameterPpi;
>
> +
>
>  STATIC ARM_CORE_INFO mCoreInfoTable[] = {
>
>    { 0x0, 0x0 }, // Cluster 0, Core 0
>
>    { 0x0, 0x1 }, // Cluster 0, Core 1
>
> @@ -46,6 +50,7 @@ ArmPlatformInitialize (
>    IN     UINTN                  MpId
>
>    )
>
>  {
>
> +  mNeoverseN1SocParameterPpi.NtFwConfig = (VOID *)gArgNtFwConfigDtPtr;
>
>    return RETURN_SUCCESS;
>
>  }
>
>
>
> @@ -80,6 +85,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
>      EFI_PEI_PPI_DESCRIPTOR_PPI,
>
>      &gArmMpCoreInfoPpiGuid,
>
>      &mMpCoreInfoPpi
>
> +  },
>
> +  {
>
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
>
> +    &gArmNeoverseN1SocParameterPpiGuid,
>
> +    &mNeoverseN1SocParameterPpi
>
>    }
>
>  };
>
>
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> index 9e8a1efc557d..d6ef92677231 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> @@ -1,6 +1,6 @@
>  /** @file
>
>
>
> -  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> @@ -10,6 +10,7 @@
>  #include <Library/DebugLib.h>
>
>  #include <Library/HobLib.h>
>
>  #include <Library/MemoryAllocationLib.h>
>
> +#include <Library/PeiServicesLib.h>
>
>  #include <NeoverseN1Soc.h>
>
>
>
>  // The total number of descriptors, including the final "end-of-table" descriptor.
>
> @@ -30,15 +31,31 @@ ArmPlatformGetVirtualMemoryMap (
>    IN     ARM_MEMORY_REGION_DESCRIPTOR  **VirtualMemoryMap
>
>    )
>
>  {
>
> -  UINTN                         Index;
>
> -  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
>
> -  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
>
> -  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
>
> -  UINT64                        DramBlock2Size;
>
> -  UINT64                        RemoteDdrSize;
>
> +  UINTN                          Index;
>
> +  ARM_MEMORY_REGION_DESCRIPTOR   *VirtualMemoryTable;
>
> +  EFI_RESOURCE_ATTRIBUTE_TYPE    ResourceAttributes;
>
> +  CONST NEOVERSEN1SOC_PLAT_INFO  *PlatInfo;
>
> +  UINT64                         DramBlock2Size;
>
> +  UINT64                         RemoteDdrSize;
>

Any reason to introduce this diff in this patch?

> +  EFI_STATUS                     Status;
>
>
>
>    Index = 0;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO
> *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
> +
>
> +  Status = PeiServicesLocatePpi (
>
> +             &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid,
>
> +             0,
>
> +             NULL,
>
> +             (VOID **)&PlatInfo
>
> +             );
>
> +  if (EFI_ERROR (Status)) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to locate gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
> - %r\n",
>
> +      gEfiCallerBaseName,
>
> +      Status
>
> +      ));
>
> +  }
>
> +

If the PPI is not found, the data in PlatInfo is not valid. So on a error above, would it be better to abort the boot?

>
>    DramBlock2Size = ((UINT64)(PlatInfo->LocalDdrSize -
>
>                               NEOVERSEN1SOC_DRAM_BLOCK1_SIZE / SIZE_1GB) *
>
>                              (UINT64)SIZE_1GB);
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> index 8d2069dea837..cbbe7aae9858 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> @@ -1,6 +1,6 @@
>  /** @file
>
>  *
>
> -*  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
>
> +*  Copyright (c) 2019 - 2023, ARM Limited. All rights reserved.
>
>  *
>
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  *
>
> @@ -25,6 +25,8 @@ GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
>  // the UEFI firmware through the CPU registers.
>
>  //
>
>  ASM_PFX(ArmPlatformPeiBootAction):
>
> +  adr  x10, gArgNtFwConfigDtPtr
>
> +  str  x0, [x10]
>
>    ret
>
>
>
>  //
>
> --
> 2.25.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#107975): https://edk2.groups.io/g/devel/message/107975
> Mute This Topic: https://groups.io/mt/100912169/1785010
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [thomas.abraham@arm.com]
> -=-=-=-=-=-=
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112180): https://edk2.groups.io/g/devel/message/112180
Mute This Topic: https://groups.io/mt/100912169/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG
  2023-08-23 11:03 [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG sahil
  2023-10-11  5:25 ` chandni cherukuri
  2023-12-07 10:52 ` Thomas Abraham
@ 2023-12-12 17:10 ` Sami Mujawar
  2 siblings, 0 replies; 4+ messages in thread
From: Sami Mujawar @ 2023-12-12 17:10 UTC (permalink / raw)
  To: sahil, devel; +Cc: Ard Biesheuvel, Leif Lindholm, nd@arm.com

[-- Attachment #1: Type: text/plain, Size: 27706 bytes --]

Hi Sahil,

Thank you for this patch.

I think this patch can be split into 3 or 4 patches. Can you look into 
that, please?

I also have other feedback marked inline as [SAMI].

Regards,

Sami Mujawar

On 23/08/2023 12:03 pm, sahil wrote:
> NT_FW_CONFIG DTB contains platform information passed by
> Tf-A boot stage.
> This information is used for Virtual memory map generation
> during PEI phase and passed on to DXE phase as a HOB, where
> it is used in ConfigurationManagerDxe.
>
> This patch adds a PEI to parse NT_FW_CONFIG and pass it to
> other PEI modules(as PPI) and DXE modules(as HOB).
>
> Signed-off-by: sahil<sahil@arm.com>
> ---
>
> Notes:
>      v3:
>       - Fixed code review comments
>       - Added a PEI to parse nt_fw_config
>
>   Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec                                                 |   8 +-
>   Platform/ARM/N1Sdp/N1SdpPlatform.dsc                                                        |   5 +-
>   Platform/ARM/N1Sdp/N1SdpPlatform.fdf                                                        |   3 +-
>   Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |   6 +-
>   Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf                             |  41 ++++++
>   Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf                               |   8 +-
>   Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h                                           |  14 +--
>   Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      |  24 ++--
>   Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c                               | 133 ++++++++++++++++++++
>   Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c                                 |  12 +-
>   Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c                              |  33 +++--
>   Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S                              |   4 +-
>   12 files changed, 255 insertions(+), 36 deletions(-)
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> index d59f25a5b915..7118da25dce0 100644
> --- a/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> +++ b/Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
> @@ -1,7 +1,7 @@
>   ## @file
>
>   #  Describes the entire platform configuration.
>
>   #
>
> -#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>   #
>
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   #
>
> @@ -22,6 +22,8 @@
>     Include                        # Root include for the package
>
>   
>
>   [Guids.common]
>
> +  # ARM NeoverseN1Soc Platform Info descriptor
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid = { 0x9fa16eb5, 0xce13, 0x4d37, { 0x96, 0xf0, 0x0a, 0xb5, 0xf1, 0xab, 0xff, 0x01 } }
>
>     gArmNeoverseN1SocTokenSpaceGuid = { 0xab93eb78, 0x60d7, 0x4099, { 0xac, 0xeb, 0x6d, 0xb5, 0x02, 0x58, 0x7c, 0x24 } }
>
>   
>
>   [PcdsFixedAtBuild]
>
> @@ -83,3 +85,7 @@
>     gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio32Translation|0x40000000000|UINT64|0x0000004F
>
>     gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation|0x40000000000|UINT64|0x00000050
>
>     gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber|2|UINT32|0x00000051
>
> +
>
> +[Ppis]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid = { 0x21D04AD4, 0x4D23, 0x426D, { 0x8D, 0x3E, 0x92, 0x23, 0x3E, 0xF4, 0xB9, 0x5E } }
>
> +  gArmNeoverseN1SocParameterPpiGuid = { 0x4DDD5A72, 0x31AD, 0x4B20, { 0x8F, 0x5F, 0xB3, 0xE8, 0x24, 0x6F, 0x80, 0x2B } }
>
> diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> index d04b22d3ef51..691d1af3c25c 100644
> --- a/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.dsc
> @@ -4,7 +4,7 @@
>   # This provides platform specific component descriptions and libraries that
>
>   # conform to EFI/Framework standards.
>
>   #
>
> -# Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +# Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>   #
>
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   #
>
> @@ -228,6 +228,9 @@
>     # Platform driver
>
>     Platform/ARM/N1Sdp/Drivers/PlatformDxe/PlatformDxe.inf
>
>   
>
> +  # PEI Phase modules
>
> +  Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
>
> +
>
>     # Human Interface Support
>
>     MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>
>   
>
> diff --git a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> index e5e24ea50294..baac8a3ef727 100644
> --- a/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> +++ b/Platform/ARM/N1Sdp/N1SdpPlatform.fdf
> @@ -1,7 +1,7 @@
>   ## @file
>
>   #  FDF file of N1Sdp
>
>   #
>
> -#  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>   #
>
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   ##
>
> @@ -195,6 +195,7 @@ READ_LOCK_STATUS   = TRUE
>     INF MdeModulePkg/Core/Pei/PeiMain.inf
>
>     INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>
>     INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>
> +  INF Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
>
>   
>
>     FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>
>       SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> index 4f8e7f13021a..242490caf860 100644
> --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> @@ -1,7 +1,7 @@
>   ## @file
>
>   #  Configuration Manager Dxe
>
>   #
>
> -#  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
>
>   #
>
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   #
>
> @@ -42,6 +42,7 @@
>   
>
>   [LibraryClasses]
>
>     ArmPlatformLib
>
> +  HobLib
>
>     PrintLib
>
>     UefiBootServicesTableLib
>
>     UefiDriverEntryPoint
>
> @@ -170,5 +171,8 @@
>     gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieMmio64Translation
>
>     gArmNeoverseN1SocTokenSpaceGuid.PcdRemotePcieSegmentNumber
>
>   
>
> +[Guids]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid
>
> +
>
>   [Depex]
>
>     TRUE
>
> diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> new file mode 100644
> index 000000000000..2eec8c327205
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.inf
> @@ -0,0 +1,41 @@
> +## @file
>
> +#  This PEI module parse the NtFwConfig for N1Sdp platform and produce
>
> +#  the PPI and HOB.
>
> +#
>
> +#  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
>
> +#
>
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +#
>
> +##
>
> +
>
> +[Defines]
>
> +  INF_VERSION                    = 0x0001001B
>
> +  BASE_NAME                      = N1SdpNtFwConfigPei
>
> +  FILE_GUID                      = CE76D56C-D3A5-4763-9138-DF09E1D1B614
>
> +  MODULE_TYPE                    = PEIM
>
> +  VERSION_STRING                 = 1.0
>
> +  ENTRY_POINT                    = Load
>
> +
>
> +[Sources]
>
> +  NtFwConfigPei.c
>
> +
>
> +[Packages]
>
> +  EmbeddedPkg/EmbeddedPkg.dec
>
> +  MdePkg/MdePkg.dec
>
> +  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec
>
> +
>
> +[LibraryClasses]
>
> +  DebugLib
>
> +  FdtLib
>
> +  HobLib
>
> +  PeimEntryPoint
>
> +
>
> +[Ppis]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
>
> +  gArmNeoverseN1SocParameterPpiGuid
>
> +
>
> +[Guids]
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorGuid
>
> +
>
> +[Depex]
>
> +  gArmNeoverseN1SocParameterPpiGuid
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> index 96e590cdd810..6e7e16b86838 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> @@ -1,7 +1,7 @@
>   ## @file
>
>   #  Platform Library for N1Sdp.
>
>   #
>
> -#  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
>
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>   #
>
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   #
>
> @@ -58,8 +58,8 @@
>   
>
>     gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
>   
>
> -[Guids]
>
> -  gEfiHobListGuid          ## CONSUMES  ## SystemTable
>
> -
>
>   [Ppis]
>
>     gArmMpCoreInfoPpiGuid
>
> +  gArmNeoverseN1SocParameterPpiGuid
>
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
>
> +
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> index 097160c7e2d1..2f83d582ccf3 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> +++ b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> @@ -1,6 +1,6 @@
>   /** @file
>
>   *
>
> -* Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
>
> +* Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
>
>   *
>
>   * SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   *
>
> @@ -41,11 +41,6 @@
>   #define NEOVERSEN1SOC_EXP_PERIPH_BASE0               0x1C000000
>
>   #define NEOVERSEN1SOC_EXP_PERIPH_BASE0_SZ            0x1300000
>
>   
>
> -// Base address to a structure of type NEOVERSEN1SOC_PLAT_INFO which is
>
> -// pre-populated by a earlier boot stage
>
> -#define NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE          (NEOVERSEN1SOC_NON_SECURE_SRAM_BASE + \
>
> -                                                      0x00008000)
>
> -
>
>   /*
>
>    * Platform information structure stored in Non-secure SRAM. Platform
>
>    * information are passed from the trusted firmware with the below structure
>
> @@ -55,12 +50,15 @@
>   typedef struct {
>
>     /*! 0 - Single Chip, 1 - Chip to Chip (C2C) */
>
>     UINT8   MultichipMode;
>
> -  /*! Slave count in C2C mode */
>
> -  UINT8   SlaveCount;
>
> +  /*! Secondary chip count in C2C mode */
>
> +  UINT8   SecondaryChipCount;
>
>     /*! Local DDR memory size in GigaBytes */
>
>     UINT8   LocalDdrSize;
>
>     /*! Remote DDR memory size in GigaBytes */
>
>     UINT8   RemoteDdrSize;
>
>   } NEOVERSEN1SOC_PLAT_INFO;
>
>   
>
> +typedef struct {
>
> +  CONST VOID    *NtFwConfig;
>
> +} NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI;
>
>   #endif
>
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index fa6408a7dd1e..e248826ec925 100644
> --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -1,7 +1,7 @@
>   /** @file
>
>     Configuration Manager Dxe
>
>   
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
>
> +  Copyright (c) 2021 - 2023, ARM Limited. All rights reserved.<BR>
>
>   
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
> @@ -16,6 +16,7 @@
>   #include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
>
>   #include <Library/ArmLib.h>
>
>   #include <Library/DebugLib.h>
>
> +#include <Library/HobLib.h>
>
>   #include <Library/IoLib.h>
>
>   #include <Library/PcdLib.h>
>
>   #include <Library/UefiBootServicesTableLib.h>
>
> @@ -28,6 +29,7 @@
>   #include "Platform.h"
>
>   
>
>   extern struct EFI_ACPI_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE Hmat;
>
> +static NEOVERSEN1SOC_PLAT_INFO *PlatInfo;

[SAMI] static should be STATIC and PlatInfo should be mPlatInfo (where m 
is prefix for module level variables).

Also have you considered making 'NEOVERSEN1SOC_PLAT_INFO *PlatInfo;' a 
field in EDKII_PLATFORM_REPOSITORY_INFO and intialising it in 
InitialisePlatformRepository().

[/SAMI]

>
>   
>
>   /** The platform configuration repository information.
>
>   */
>
> @@ -1242,13 +1244,21 @@ InitializePlatformRepository (
>     IN  EDKII_PLATFORM_REPOSITORY_INFO  * CONST PlatRepoInfo
>
>     )
>
>   {
>
> -  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
>
>     UINT64                        Dram2Size;
>
>     UINT64                        RemoteDdrSize;
>
> +  VOID                          *PlatInfoHob;
>
> +
>
> +  PlatInfoHob = GetFirstGuidHob (&gArmNeoverseN1SocPlatformInfoDescriptorGuid);
>
> +
>
> +  if (PlatInfoHob == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
>
> +    return EFI_NOT_FOUND;
>
> +  }
>
> +
>
> +  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)GET_GUID_HOB_DATA (PlatInfoHob);
>
>   
>
>     RemoteDdrSize = 0;
>
>   
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>     Dram2Size = ((PlatInfo->LocalDdrSize - 2) * SIZE_1GB);
>
>   
>
>     PlatRepoInfo->MemAffInfo[LOCAL_DDR_REGION2].Length = Dram2Size;
>
> @@ -1512,7 +1522,6 @@ GetGicCInfo (
>     )
>
>   {
>
>     EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
>
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>
>     UINT32                            TotalObjCount;
>
>     UINT32                            ObjIndex;
>
>   
>
> @@ -1523,7 +1532,6 @@ GetGicCInfo (
>     }
>
>   
>
>     PlatformRepo = This->PlatRepoInfo;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>   
>
>     if (PlatInfo->MultichipMode == 1) {
>
>       TotalObjCount = PLAT_CPU_COUNT * 2;
>
> @@ -1623,7 +1631,6 @@ GetStandardNameSpaceObject (
>   {
>
>     EFI_STATUS                        Status;
>
>     EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
>
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>
>     UINT32                            AcpiTableCount;
>
>   
>
>     if ((This == NULL) || (CmObject == NULL)) {
>
> @@ -1634,7 +1641,7 @@ GetStandardNameSpaceObject (
>   
>
>     Status = EFI_NOT_FOUND;
>
>     PlatformRepo = This->PlatRepoInfo;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
> +
>
>     AcpiTableCount = ARRAY_SIZE (PlatformRepo->CmAcpiTableList);
>
>     if (PlatInfo->MultichipMode == 0)
>
>           AcpiTableCount -= 1;
>
> @@ -1697,7 +1704,6 @@ GetArmNameSpaceObject (
>   {
>
>     EFI_STATUS                        Status;
>
>     EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
>
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>
>     UINT32                            GicRedistCount;
>
>     UINT32                            GicCpuCount;
>
>     UINT32                            ProcHierarchyInfoCount;
>
> @@ -1718,8 +1724,6 @@ GetArmNameSpaceObject (
>     Status = EFI_NOT_FOUND;
>
>     PlatformRepo = This->PlatRepoInfo;
>
>   
>
> -  // Probe for multi chip information
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
>     if (PlatInfo->MultichipMode == 1) {
>
>       GicRedistCount = 2;
>
>       GicCpuCount = PLAT_CPU_COUNT * 2;
>
> diff --git a/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> new file mode 100644
> index 000000000000..ff6a6a7933c3
> --- /dev/null
> +++ b/Platform/ARM/N1Sdp/Drivers/N1SdpNtFwConfigPei/NtFwConfigPei.c
> @@ -0,0 +1,133 @@
> +/** @file
>
> +
>
> +  Copyright (c) 2023, ARM Limited. All rights reserved.<BR>
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#include <Library/DebugLib.h>
>
> +#include <Library/HobLib.h>
>
> +#include <Library/PeiServicesLib.h>
>
> +
>
> +#include <NeoverseN1Soc.h>
>
> +#include <libfdt.h>
>
> +
>
> +STATIC EFI_PEI_PPI_DESCRIPTOR  gPpi;
[SAMI] The prefix should be 'm' not 'g'. 'g' is for global, which I do 
not think is your intention here.
>
> +
>
> +/**
>
> +  The entrypoint of the module, parse NtFwConfig and produce the PPI and HOB.
>
> +
>
> +  @param[in]  FileHandle   Handle of the file being invoked.
>
> +  @param[in]  PeiServices  Describes the list of possible PEI Services.
>
> +
>
> +  @retval EFI_SUCCESS      Either no NT_FW_CONFIG was given by EL3 firmware
>
> +                           OR the N1Sdp FDT HOB was successfully created.
>
> +  @retval EFI_UNSUPPORTED  FDT header sanity check failed.
>
> +  @retval *                Other errors are possible.
[SAMI] Please list the other possible values. At least the ones that are 
returned by this function.
>
> +**/
>
> +
>
> +EFI_STATUS
>
> +EFIAPI
>
> +Load (
[SAMI] Is it possible to change the name of the function to something 
like NtFwConfigPeEntryPoint() ?
>
> +    IN EFI_PEI_FILE_HANDLE     FileHandle,
>
> +    IN CONST EFI_PEI_SERVICES  **PeiServices
>
> +  )
>
> +{
>
> +  CONST NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI *ParamPpi;
>
> +  CONST UINT32                                 *Property;
>
> +  INT32                                        Offset;
>
> +  NEOVERSEN1SOC_PLAT_INFO                      *PlatInfo;
>
> +  INT32                                        Status;
>
> +
>
> +  PlatInfo = BuildGuidHob (
>
> +               &gArmNeoverseN1SocPlatformInfoDescriptorGuid,
>
> +               sizeof (*PlatInfo)
>
> +               );
>
> +
>
> +  if (PlatInfo == NULL) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to allocate platform info HOB\n",
>
> +      gEfiCallerBaseName
>
> +      ));
>
> +    return EFI_INVALID_PARAMETER;
[SAMI] EFI_OUT_OF_RESOURCES ?
>
> +  }
>
> +
>
> +  Status = PeiServicesLocatePpi (
>
> +             &gArmNeoverseN1SocParameterPpiGuid,
>
> +             0,
>
> +             NULL,
>
> +             (VOID **)&ParamPpi
>
> +             );
>
> +
>
> +  if (EFI_ERROR (Status)) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to locate gArmNeoverseN1SocParameterPpiGuid - %r\n",
>
> +      gEfiCallerBaseName,
>
> +      Status
>
> +      ));
>
> +    return EFI_INVALID_PARAMETER;
[SAMI] Why not return the 'Status' that was returned?
>
> +  }
>
> +
>
> +  if (fdt_check_header (ParamPpi->NtFwConfig) != 0) {
>
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", ParamPpi->NtFwConfig));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  Offset = fdt_subnode_offset (ParamPpi->NtFwConfig, 0, "platform-info");
>
> +  if (Offset == -FDT_ERR_NOTFOUND) {
>
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB : platform-info node not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
[SAMI] EFI_NOT_FOUND ?
>
> +  }
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "local-ddr-size", NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "local-ddr-size property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
[SAMI] EFI_NOT_FOUND ? Similar change at other places in this file.
>
> +  }
>
> +
>
> +  PlatInfo->LocalDdrSize = fdt32_to_cpu (*Property);
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "remote-ddr-size", NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "remote-ddr-size property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->RemoteDdrSize = fdt32_to_cpu (*Property);
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "secondary-chip-count", NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "secondary-chip-count property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->SecondaryChipCount = fdt32_to_cpu (*Property);
>
> +
>
> +  Property = fdt_getprop (ParamPpi->NtFwConfig, Offset, "multichip-mode", NULL);
>
> +  if (Property == NULL) {
>
> +    DEBUG ((DEBUG_ERROR, "multichip-mode property not found\n"));
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PlatInfo->MultichipMode = fdt32_to_cpu (*Property);
>
> +
>
> +  gPpi.Flags = EFI_PEI_PPI_DESCRIPTOR_PPI
>
> +               | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
>
> +  gPpi.Guid = &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid;
>
> +  gPpi.Ppi  = PlatInfo;
>
> +
>
> +  Status = PeiServicesInstallPpi (&gPpi);
>
> +  if (EFI_ERROR (Status)) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to install PEI service - %r\n",
>
> +      gEfiCallerBaseName,
>
> +      Status
>
> +      ));
>
> +    return EFI_INVALID_PARAMETER;
[SAMI] Drop the return statement in the line above and return 'Status' 
in the line after the if block below.
>
> +  }
>
> +
>
> +  return EFI_SUCCESS;
>
> +}
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> index c0effd37f333..52ee6299b3fe 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.c
> @@ -1,6 +1,6 @@
>   /** @file
>
>   
>
> -  Copyright (c) 2018-2021, ARM Limited. All rights reserved.<BR>
>
> +  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>   
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
> @@ -8,8 +8,12 @@
>   
>
>   #include <Library/ArmPlatformLib.h>
>
>   #include <Library/BaseLib.h>
>
> +#include <NeoverseN1Soc.h>
>
>   #include <Ppi/ArmMpCoreInfo.h>
>
>   
>
> +UINT64  gArgNtFwConfigDtPtr;
>
> +STATIC  NEOVERSEN1SOC_EL3_FW_HANDOFF_PARAM_PPI mNeoverseN1SocParameterPpi;
>
> +
>
>   STATIC ARM_CORE_INFO mCoreInfoTable[] = {
>
>     { 0x0, 0x0 }, // Cluster 0, Core 0
>
>     { 0x0, 0x1 }, // Cluster 0, Core 1
>
> @@ -46,6 +50,7 @@ ArmPlatformInitialize (
>     IN     UINTN                  MpId
>
>     )
>
>   {
>
> +  mNeoverseN1SocParameterPpi.NtFwConfig = (VOID *)gArgNtFwConfigDtPtr;
>
>     return RETURN_SUCCESS;
>
>   }
>
>   
>
> @@ -80,6 +85,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
>       EFI_PEI_PPI_DESCRIPTOR_PPI,
>
>       &gArmMpCoreInfoPpiGuid,
>
>       &mMpCoreInfoPpi
>
> +  },
>
> +  {
>
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
>
> +    &gArmNeoverseN1SocParameterPpiGuid,
>
> +    &mNeoverseN1SocParameterPpi
>
>     }
>
>   };
>
>   
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> index 9e8a1efc557d..d6ef92677231 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> @@ -1,6 +1,6 @@
>   /** @file
>
>   
>
> -  Copyright (c) 2018 - 2021, ARM Limited. All rights reserved.<BR>
>
> +  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.<BR>
>
>   
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
> @@ -10,6 +10,7 @@
>   #include <Library/DebugLib.h>
>
>   #include <Library/HobLib.h>
>
>   #include <Library/MemoryAllocationLib.h>
>
> +#include <Library/PeiServicesLib.h>
>
>   #include <NeoverseN1Soc.h>
>
>   
>
>   // The total number of descriptors, including the final "end-of-table" descriptor.
>
> @@ -30,15 +31,31 @@ ArmPlatformGetVirtualMemoryMap (
>     IN     ARM_MEMORY_REGION_DESCRIPTOR  **VirtualMemoryMap
>
>     )
>
>   {
>
> -  UINTN                         Index;
>
> -  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
>
> -  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
>
> -  NEOVERSEN1SOC_PLAT_INFO       *PlatInfo;
>
> -  UINT64                        DramBlock2Size;
>
> -  UINT64                        RemoteDdrSize;
>
> +  UINTN                          Index;
>
> +  ARM_MEMORY_REGION_DESCRIPTOR   *VirtualMemoryTable;
>
> +  EFI_RESOURCE_ATTRIBUTE_TYPE    ResourceAttributes;
>
> +  CONST NEOVERSEN1SOC_PLAT_INFO  *PlatInfo;
>
> +  UINT64                         DramBlock2Size;
>
> +  UINT64                         RemoteDdrSize;
>
> +  EFI_STATUS                     Status;
>
>   
>
>     Index = 0;
>
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
> +
>
> +  Status = PeiServicesLocatePpi (
>
> +             &gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid,
>
> +             0,
>
> +             NULL,
>
> +             (VOID **)&PlatInfo
>
> +             );
>
> +  if (EFI_ERROR (Status)) {
>
> +    DEBUG ((
>
> +      DEBUG_ERROR,
>
> +      "[%a]: failed to locate gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid - %r\n",
>
> +      gEfiCallerBaseName,
>
> +      Status
>
> +      ));

[SAMI] I think the code should not continue otherwise dereferencing the 
PlatInfo pointer will result in a crash.

It may also be useful if you set *VirtualMemoryMap = NULL.

I acknowledge the problem would not be fixed entirely, as the calling 
function does not handle this. But at least this function would be doing 
the right thing.

Ideally we need ArmPlatformGetVirtualMemoryMap() to return an error code 
and the calling function(s) should handle the errors appropriately. 
However, that is a bigger change and not part of this patch series.

[/SAMI]

>
> +  }
>
> +
>
>     DramBlock2Size = ((UINT64)(PlatInfo->LocalDdrSize -
>
>                                NEOVERSEN1SOC_DRAM_BLOCK1_SIZE / SIZE_1GB) *
>
>                               (UINT64)SIZE_1GB);
>
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> index 8d2069dea837..cbbe7aae9858 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/AArch64/Helper.S
> @@ -1,6 +1,6 @@
>   /** @file
>
>   *
>
> -*  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
>
> +*  Copyright (c) 2019 - 2023, ARM Limited. All rights reserved.
>
>   *
>
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   *
>
> @@ -25,6 +25,8 @@ GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
>   // the UEFI firmware through the CPU registers.
>
>   //
>
>   ASM_PFX(ArmPlatformPeiBootAction):
>
> +  adr  x10, gArgNtFwConfigDtPtr
>
> +  str  x0, [x10]
>
>     ret
>
>   
>
>   //
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112452): https://edk2.groups.io/g/devel/message/112452
Mute This Topic: https://groups.io/mt/100912169/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 28145 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-12-12 17:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-23 11:03 [edk2-devel] [edk2-platforms][PATCH V3 1/1] Platform/ARM/N1sdp: Add support to parse NT_FW_CONFIG sahil
2023-10-11  5:25 ` chandni cherukuri
2023-12-07 10:52 ` Thomas Abraham
2023-12-12 17:10 ` Sami Mujawar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox