Re: [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG

[Thomas Abraham <thomas.abraham@arm.com>]

Hi Ard, Leif,

On Tue, Dec 4, 2018 at 5:31 PM Chandni Cherukuri
<chandni.cherukuri@arm.com> wrote:
>
> The hardware configuration in HW_CONFIG dts is not being
> passed onto the operating system but used and terminated
> at edk2 boot stage (BL33). So, as per the recommendations
> of the trusted-firmware design, the hardware description
> specified in NT_FW_CONFIG dts should be used instead of
> HW_CONFIG dts.

The corresponding change in upstream trusted firmware has been merged.
So can you please have a look at this patch and let us know if any
changes are needed. Until this patch gets merged, the upstream master
branch of trusted firmware and edk2-platforms support for SGI
platforms is out of sync with each other. We will take care next time
to avoid causing dependencies like these between these two repos.

Thanks,
Thomas.

>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> ---
>  Platform/ARM/SgiPkg/SgiPlatform.dec                           |  2 +-
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf       |  2 +-
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf |  4 +--
>  Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h               |  6 ++---
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c         | 10 +++----
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c  | 28 ++++++++++----------
>  Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  6 ++---
>  7 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> index f6e0ba1..9166052 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -45,4 +45,4 @@
>    gArmSgiTokenSpaceGuid.PcdDramBlock2Size|0|UINT64|0x00000003
>
>  [Ppis]
> -  gHwConfigDtInfoPpiGuid     = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> +  gNtFwConfigDtInfoPpiGuid     = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> index 93377fa..260be58 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> @@ -71,4 +71,4 @@
>
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
> -  gHwConfigDtInfoPpiGuid
> +  gNtFwConfigDtInfoPpiGuid
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
> index a7c718b..a9173cc 100644
> --- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
> @@ -34,7 +34,7 @@
>    gArmSgiPlatformIdDescriptorGuid
>
>  [Ppis]
> -  gHwConfigDtInfoPpiGuid
> +  gNtFwConfigDtInfoPpiGuid
>
>  [Depex]
> -  gHwConfigDtInfoPpiGuid
> +  gNtFwConfigDtInfoPpiGuid
> diff --git a/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
> index 934eef2..b830326 100644
> --- a/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
> +++ b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
> @@ -15,9 +15,9 @@
>  #ifndef  SGI_PLATFORMID_PPI_
>  #define  SGI_PLATFORMID_PPI_
>
> -// HwConfig DT structure
> +// NT_FW_CONFIG DT structure
>  typedef struct {
> -  UINT64                  HwConfigDtAddr;
> -} SGI_HW_CONFIG_INFO_PPI;
> +  UINT64                  NtFwConfigDtAddr;
> +} SGI_NT_FW_CONFIG_INFO_PPI;
>
>  #endif
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> index 13bb423..d264292 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> @@ -17,8 +17,8 @@
>  #include <Ppi/ArmMpCoreInfo.h>
>  #include <Ppi/SgiPlatformId.h>
>
> -UINT64 HwConfigDtBlob;
> -STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
> +UINT64 NtFwConfigDtBlob;
> +STATIC SGI_NT_FW_CONFIG_INFO_PPI mNtFwConfigDtInfoPpi;
>
>  STATIC ARM_CORE_INFO mCoreInfoTable[] = {
>    {
> @@ -40,7 +40,7 @@ ArmPlatformInitialize (
>    IN  UINTN                     MpId
>    )
>  {
> -  mHwConfigDtInfoPpi.HwConfigDtAddr = HwConfigDtBlob;
> +  mNtFwConfigDtInfoPpi.NtFwConfigDtAddr = NtFwConfigDtBlob;
>    return RETURN_SUCCESS;
>  }
>
> @@ -67,8 +67,8 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
>    },
>    {
>      EFI_PEI_PPI_DESCRIPTOR_PPI,
> -    &gHwConfigDtInfoPpiGuid,
> -    &mHwConfigDtInfoPpi
> +    &gNtFwConfigDtInfoPpiGuid,
> +    &mNtFwConfigDtInfoPpi
>    }
>  };
>
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> index 065b23d..d133921 100644
> --- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> @@ -24,9 +24,9 @@
>
>    This function returns the System ID of the platform
>
> -  This functions locates the HwConfig PPI and gets the base address of HW CONFIG
> -  DT from which the platform ID and config ID is obtained using FDT helper
> -  functions
> +  This functions locates the NtFwConfig PPI and gets the base address of
> +  NT_FW_CONFIG DT from which the platform ID and config ID is obtained
> +  using FDT helper functions
>
>    @param[out]      Pointer to the SGI_PLATFORM_DESCRIPTOR HoB
>    @return          returns EFI_SUCCESS on success and EFI_INVALID_PARAMETER on error
> @@ -41,31 +41,31 @@ GetSgiSystemId (
>  {
>    CONST UINT32                  *Property;
>    INT32                         Offset;
> -  CONST VOID                    *HwCfgDtBlob;
> -  SGI_HW_CONFIG_INFO_PPI        *HwConfigInfoPpi;
> +  CONST VOID                    *NtFwCfgDtBlob;
> +  SGI_NT_FW_CONFIG_INFO_PPI     *NtFwConfigInfoPpi;
>    EFI_STATUS                    Status;
>
> -  Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL,
> -             (VOID**)&HwConfigInfoPpi);
> +  Status = PeiServicesLocatePpi (&gNtFwConfigDtInfoPpiGuid, 0, NULL,
> +             (VOID**)&NtFwConfigInfoPpi);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR,
>        "PeiServicesLocatePpi failed with error %r\n", Status));
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  HwCfgDtBlob = (VOID *)(UINTN)HwConfigInfoPpi->HwConfigDtAddr;
> -  if (fdt_check_header (HwCfgDtBlob) != 0) {
> -    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", HwCfgDtBlob));
> +  NtFwCfgDtBlob = (VOID *)(UINTN)NtFwConfigInfoPpi->NtFwConfigDtAddr;
> +  if (fdt_check_header (NtFwCfgDtBlob) != 0) {
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\n", NtFwCfgDtBlob));
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id");
> +  Offset = fdt_subnode_offset (NtFwCfgDtBlob, 0, "system-id");
>    if (Offset == -FDT_ERR_NOTFOUND) {
>      DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
> +  Property = fdt_getprop (NtFwCfgDtBlob, Offset, "platform-id", NULL);
>    if (Property == NULL) {
>      DEBUG ((DEBUG_ERROR, "platform-id property not found\n"));
>      return EFI_INVALID_PARAMETER;
> @@ -73,7 +73,7 @@ GetSgiSystemId (
>
>    HobData->PlatformId = fdt32_to_cpu (*Property);
>
> -  Property = fdt_getprop (HwCfgDtBlob, Offset, "config-id", NULL);
> +  Property = fdt_getprop (NtFwCfgDtBlob, Offset, "config-id", NULL);
>    if (Property == NULL) {
>      DEBUG ((DEBUG_ERROR, "config-id property not found\n"));
>      return EFI_INVALID_PARAMETER;
> @@ -108,7 +108,7 @@ SgiPlatformPeim (
>                                           &gArmSgiPlatformIdDescriptorGuid,
>                                           sizeof (SGI_PLATFORM_DESCRIPTOR));
>
> -  // Get the system id from the platform specific hw_config device tree
> +  // Get the system id from the platform specific nt_fw_config device tree
>    if (HobData == NULL) {
>      DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
>      ASSERT (FALSE);
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> index 3662266..5dc6431 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> @@ -22,7 +22,7 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
>  GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
>  GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
>  GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> -GCC_ASM_IMPORT(HwConfigDtBlob)
> +GCC_ASM_IMPORT(NtFwConfigDtBlob)
>
>  //
>  // First platform specific function to be called in the PEI phase
> @@ -34,8 +34,8 @@ GCC_ASM_IMPORT(HwConfigDtBlob)
>  //  VOID
>  //  );
>  ASM_PFX(ArmPlatformPeiBootAction):
> -  adr  x10, HwConfigDtBlob
> -  str  x1, [x10]
> +  adr  x10, NtFwConfigDtBlob
> +  str  x0, [x10]
>    ret
>
>  //UINTN
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel