Re: [edk2-devel] [edk2-platforms][PATCH V4 4/4] Silicon/ARM/NeoverseN1Soc: Consume N1SdpNtFwConfigPei supplied data

["sahil" <sahil@arm.com>]

Hi All,

Please find the links to previous discussions below :

V1 -
https://edk2.groups.io/g/devel/topic/96088980#100022
V2 -
https://edk2.groups.io/g/devel/topic/96671861#103652
V3 -
https://edk2.groups.io/g/devel/topic/100912169#112452

Thanks,
Sahil


On Thu, 4 Jan 2024 at 18:46, sahil <sahil@arm.com> wrote:
>
> N1SdpNtFwConfigPei PEIM extracts platform information from NT_FW_CONFIG
> and provides it to other modules as a PPI and a HOB. PlatformLibMem
> then consumes these values in the form of a PPI during PEI phase and
> ConfigurationManagerDxe as a HOB during DXE phase. The previous
> approach of fetching platform information from a fixed address
> is also removed in this patch.
>
> Signed-off-by: sahil <sahil@arm.com>
> ---
>  Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |  6 +++-
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf                               |  1 +
>  Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h      |  5 ++-
>  Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h                                           |  5 ---
>  Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      | 33 +++++++++---------
>  Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c                              | 35 +++++++++++++++-----
>  6 files changed, 55 insertions(+), 30 deletions(-)
>
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> index 4f8e7f13021a..e4aaed3e9dd5 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 - 2024, 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/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> index ead7f11ec964..e7af4c711817 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> +++ b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLib.inf
> @@ -64,3 +64,4 @@
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
>    gArmNeoverseN1SocParameterPpiGuid
> +  gArmNeoverseN1SocPlatformInfoDescriptorPpiGuid
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
> index 303c38dcaee9..ea9cb20ba752 100644
> --- a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
> +++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
> @@ -1,6 +1,6 @@
>  /** @file
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2021-2024, ARM Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -302,6 +302,9 @@ typedef struct PlatformRepositoryInfo {
>    /// Memory Affinity Info
>    CM_ARM_MEMORY_AFFINITY_INFO           MemAffInfo[DDR_REGION_COUNT];
>
> +  /// N1Sdp Platform Info
> +  NEOVERSEN1SOC_PLAT_INFO               *PlatInfo;
> +
>  } EDKII_PLATFORM_REPOSITORY_INFO;
>
>  #endif // CONFIGURATION_MANAGER_H_
> diff --git a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> index 2bddf5007431..5483e7bc5f68 100644
> --- a/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> +++ b/Silicon/ARM/NeoverseN1Soc/Include/NeoverseN1Soc.h
> @@ -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
> diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index fa6408a7dd1e..966a83294c00 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 - 2024, 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>
> @@ -1242,19 +1243,27 @@ 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;
> +  }
> +
> +  PlatRepoInfo->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);
> +  Dram2Size = ((PlatRepoInfo->PlatInfo->LocalDdrSize - 2) * SIZE_1GB);
>
>    PlatRepoInfo->MemAffInfo[LOCAL_DDR_REGION2].Length = Dram2Size;
>
> -  if (PlatInfo->MultichipMode == 1) {
> -    RemoteDdrSize = ((UINT64)(PlatInfo->RemoteDdrSize - 2) * SIZE_1GB);
> +  if (PlatRepoInfo->PlatInfo->MultichipMode == 1) {
> +    RemoteDdrSize = ((UINT64)(PlatRepoInfo->PlatInfo->RemoteDdrSize - 2) * SIZE_1GB);
>
>      // Update Remote DDR Region1
>      PlatRepoInfo->MemAffInfo[REMOTE_DDR_REGION1].ProximityDomain = 1;
> @@ -1512,7 +1521,6 @@ GetGicCInfo (
>    )
>  {
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>    UINT32                            TotalObjCount;
>    UINT32                            ObjIndex;
>
> @@ -1523,9 +1531,8 @@ GetGicCInfo (
>    }
>
>    PlatformRepo = This->PlatRepoInfo;
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
>
> -  if (PlatInfo->MultichipMode == 1) {
> +  if (PlatformRepo->PlatInfo->MultichipMode == 1) {
>      TotalObjCount = PLAT_CPU_COUNT * 2;
>    } else {
>      TotalObjCount = PLAT_CPU_COUNT;
> @@ -1623,7 +1630,6 @@ GetStandardNameSpaceObject (
>  {
>    EFI_STATUS                        Status;
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>    UINT32                            AcpiTableCount;
>
>    if ((This == NULL) || (CmObject == NULL)) {
> @@ -1634,9 +1640,8 @@ 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)
> +  if (PlatformRepo->PlatInfo->MultichipMode == 0)
>          AcpiTableCount -= 1;
>
>    switch (GET_CM_OBJECT_ID (CmObjectId)) {
> @@ -1697,7 +1702,6 @@ GetArmNameSpaceObject (
>  {
>    EFI_STATUS                        Status;
>    EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> -  NEOVERSEN1SOC_PLAT_INFO           *PlatInfo;
>    UINT32                            GicRedistCount;
>    UINT32                            GicCpuCount;
>    UINT32                            ProcHierarchyInfoCount;
> @@ -1719,8 +1723,7 @@ GetArmNameSpaceObject (
>    PlatformRepo = This->PlatRepoInfo;
>
>    // Probe for multi chip information
> -  PlatInfo = (NEOVERSEN1SOC_PLAT_INFO *)NEOVERSEN1SOC_PLAT_INFO_STRUCT_BASE;
> -  if (PlatInfo->MultichipMode == 1) {
> +  if (PlatformRepo->PlatInfo->MultichipMode == 1) {
>      GicRedistCount = 2;
>      GicCpuCount = PLAT_CPU_COUNT * 2;
>      ProcHierarchyInfoCount = PLAT_PROC_HIERARCHY_NODE_COUNT * 2;
> diff --git a/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c b/Silicon/ARM/NeoverseN1Soc/Library/PlatformLib/PlatformLibMem.c
> index 9e8a1efc557d..80daedb33416 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 - 2024, 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,33 @@ 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
> +      ));
> +      *VirtualMemoryMap = NULL;
> +      return;
> +  }
> +
>    DramBlock2Size = ((UINT64)(PlatInfo->LocalDdrSize -
>                               NEOVERSEN1SOC_DRAM_BLOCK1_SIZE / SIZE_1GB) *
>                              (UINT64)SIZE_1GB);
> --
> 2.25.1
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#113167): https://edk2.groups.io/g/devel/message/113167
> Mute This Topic: https://groups.io/mt/103521648/7175337
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [sahilkaushal05@gmail.com]
> ------------
>
>


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