[PATCH edk2-platforms 0/2] Use NT_FW_CONFIG instead of HW_CONFIG

[PATCH edk2-platforms 0/2] Use NT_FW_CONFIG instead of HW_CONFIG

[Chandni Cherukuri]

The first patch removes an incorrect check on the configuration id
register value and does minor changes to the console error messages.

The second patch in this series adds usage of NT_FW_CONFIG for SGI
platforms. The hardware configuration (system-id) in HW_CONFIG dts
is not being passed onto the operating system. As per the
recommendations of the trusted-firmware design, since the hardware
description is being used only in edk2 boot stage (BL33), the
non-trusted firmware config device tree (NT_FW_CONFIG) can be used
instead of HW_CONFIG device tree to specify the hardware description.

Chandni Cherukuri (2): 
  Platform/ARM/Sgi: fix incorrect check of config-id value
  Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG

 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  | 34 ++++++++++----------
 Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  6 ++--
 7 files changed, 32 insertions(+), 32 deletions(-)

-- 
2.7.4

[PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value

[Chandni Cherukuri]

On SGI platform, the value of configuration ID can be zero.
So avoid returning an error from the function that creates
the system ID HOB in case the value of the configuration ID
is zero.

While at it, improve some of the error messages as well.

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/Library/SgiPlatformPei/SgiPlatformPeim.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
index 15ea571..065b23d 100644
--- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
+++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
@@ -67,7 +67,7 @@ GetSgiSystemId (
 
   Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
   if (Property == NULL) {
-    DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
+    DEBUG ((DEBUG_ERROR, "platform-id property not found\n"));
     return EFI_INVALID_PARAMETER;
   }
 
@@ -75,7 +75,7 @@ GetSgiSystemId (
 
   Property = fdt_getprop (HwCfgDtBlob, Offset, "config-id", NULL);
   if (Property == NULL) {
-    DEBUG ((DEBUG_ERROR, "Config Id is NULL\n"));
+    DEBUG ((DEBUG_ERROR, "config-id property not found\n"));
     return EFI_INVALID_PARAMETER;
   }
 
@@ -121,7 +121,7 @@ SgiPlatformPeim (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (HobData->PlatformId == 0 || HobData->ConfigId == 0) {
+  if (HobData->PlatformId == 0) {
     ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   }
-- 
2.7.4

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

[Chandni Cherukuri]

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.

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

Re: [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value

[Ard Biesheuvel]

On Tue, 4 Dec 2018 at 12:36, Chandni Cherukuri
<chandni.cherukuri@arm.com> wrote:
>
> On SGI platform, the value of configuration ID can be zero.
> So avoid returning an error from the function that creates
> the system ID HOB in case the value of the configuration ID
> is zero.
>
> While at it, improve some of the error messages as well.
>

So the platform ID is still guaranteed to be non-zero?


> 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/Library/SgiPlatformPei/SgiPlatformPeim.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> index 15ea571..065b23d 100644
> --- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> @@ -67,7 +67,7 @@ GetSgiSystemId (
>
>    Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
>    if (Property == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
> +    DEBUG ((DEBUG_ERROR, "platform-id property not found\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>
> @@ -75,7 +75,7 @@ GetSgiSystemId (
>
>    Property = fdt_getprop (HwCfgDtBlob, Offset, "config-id", NULL);
>    if (Property == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Config Id is NULL\n"));
> +    DEBUG ((DEBUG_ERROR, "config-id property not found\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>
> @@ -121,7 +121,7 @@ SgiPlatformPeim (
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  if (HobData->PlatformId == 0 || HobData->ConfigId == 0) {
> +  if (HobData->PlatformId == 0) {
>      ASSERT (FALSE);
>      return EFI_INVALID_PARAMETER;
>    }
> --
> 2.7.4
>

Re: [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value

[chandni cherukuri]

On Tue, Dec 4, 2018 at 8:26 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 4 Dec 2018 at 12:36, Chandni Cherukuri
> <chandni.cherukuri@arm.com> wrote:
> >
> > On SGI platform, the value of configuration ID can be zero.
> > So avoid returning an error from the function that creates
> > the system ID HOB in case the value of the configuration ID
> > is zero.
> >
> > While at it, improve some of the error messages as well.
> >
>
> So the platform ID is still guaranteed to be non-zero?
>
>

Platform ID is the part number of the platform and it is a
unique 12-bit value as specified by the SGI platform
specification. So it is guaranteed to be non-zero value.

Thanks
Chandni

> > 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/Library/SgiPlatformPei/SgiPlatformPeim.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> > index 15ea571..065b23d 100644
> > --- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> > +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> > @@ -67,7 +67,7 @@ GetSgiSystemId (
> >
> >    Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
> >    if (Property == NULL) {
> > -    DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
> > +    DEBUG ((DEBUG_ERROR, "platform-id property not found\n"));
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > @@ -75,7 +75,7 @@ GetSgiSystemId (
> >
> >    Property = fdt_getprop (HwCfgDtBlob, Offset, "config-id", NULL);
> >    if (Property == NULL) {
> > -    DEBUG ((DEBUG_ERROR, "Config Id is NULL\n"));
> > +    DEBUG ((DEBUG_ERROR, "config-id property not found\n"));
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > @@ -121,7 +121,7 @@ SgiPlatformPeim (
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > -  if (HobData->PlatformId == 0 || HobData->ConfigId == 0) {
> > +  if (HobData->PlatformId == 0) {
> >      ASSERT (FALSE);
> >      return EFI_INVALID_PARAMETER;
> >    }
> > --
> > 2.7.4
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Thomas Abraham]

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

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

[Ard Biesheuvel]

On Thu, 6 Dec 2018 at 16:50, Thomas Abraham <thomas.abraham@arm.com> wrote:
>
> 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.
>

For the series

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as 07c6bc27730d..327ff4ae71ae

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

[Thomas Abraham]

On Thu, Dec 6, 2018 at 10:25 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 6 Dec 2018 at 16:50, Thomas Abraham <thomas.abraham@arm.com> wrote:
> >
> > 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.
> >
>
> For the series
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Pushed as 07c6bc27730d..327ff4ae71ae

Thank you Ard.