* [PATCH edk2-platforms 0/2] Use NT_FW_CONFIG instead of HW_CONFIG @ 2018-12-04 11:36 Chandni Cherukuri 2018-12-04 11:36 ` [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value Chandni Cherukuri 2018-12-04 11:36 ` [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri 0 siblings, 2 replies; 8+ messages in thread From: Chandni Cherukuri @ 2018-12-04 11:36 UTC (permalink / raw) To: edk2-devel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value 2018-12-04 11:36 [PATCH edk2-platforms 0/2] Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri @ 2018-12-04 11:36 ` Chandni Cherukuri 2018-12-04 14:55 ` Ard Biesheuvel 2018-12-04 11:36 ` [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri 1 sibling, 1 reply; 8+ messages in thread From: Chandni Cherukuri @ 2018-12-04 11:36 UTC (permalink / raw) To: edk2-devel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value 2018-12-04 11:36 ` [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value Chandni Cherukuri @ 2018-12-04 14:55 ` Ard Biesheuvel 2018-12-05 4:16 ` chandni cherukuri 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2018-12-04 14:55 UTC (permalink / raw) To: Chandni Cherukuri; +Cc: edk2-devel@lists.01.org, Leif Lindholm 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value 2018-12-04 14:55 ` Ard Biesheuvel @ 2018-12-05 4:16 ` chandni cherukuri 0 siblings, 0 replies; 8+ messages in thread From: chandni cherukuri @ 2018-12-05 4:16 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Chandni Cherukuri, edk2-devel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG 2018-12-04 11:36 [PATCH edk2-platforms 0/2] Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri 2018-12-04 11:36 ` [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value Chandni Cherukuri @ 2018-12-04 11:36 ` Chandni Cherukuri 2018-12-06 15:42 ` Thomas Abraham 1 sibling, 1 reply; 8+ messages in thread From: Chandni Cherukuri @ 2018-12-04 11:36 UTC (permalink / raw) To: edk2-devel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG 2018-12-04 11:36 ` [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri @ 2018-12-06 15:42 ` Thomas Abraham 2018-12-06 16:55 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Thomas Abraham @ 2018-12-06 15:42 UTC (permalink / raw) To: chandni.cherukuri; +Cc: edk2-devel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG 2018-12-06 15:42 ` Thomas Abraham @ 2018-12-06 16:55 ` Ard Biesheuvel 2018-12-06 18:01 ` Thomas Abraham 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2018-12-06 16:55 UTC (permalink / raw) To: Thomas Panakamattam Abraham; +Cc: Chandni Cherukuri, edk2-devel@lists.01.org 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG 2018-12-06 16:55 ` Ard Biesheuvel @ 2018-12-06 18:01 ` Thomas Abraham 0 siblings, 0 replies; 8+ messages in thread From: Thomas Abraham @ 2018-12-06 18:01 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-06 18:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-04 11:36 [PATCH edk2-platforms 0/2] Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri 2018-12-04 11:36 ` [PATCH edk2-platforms 1/2] Platform/ARM/Sgi: fix incorrect check of config-id value Chandni Cherukuri 2018-12-04 14:55 ` Ard Biesheuvel 2018-12-05 4:16 ` chandni cherukuri 2018-12-04 11:36 ` [PATCH edk2-platforms 2/2] Platform/ARM/Sgi: Use NT_FW_CONFIG instead of HW_CONFIG Chandni Cherukuri 2018-12-06 15:42 ` Thomas Abraham 2018-12-06 16:55 ` Ard Biesheuvel 2018-12-06 18:01 ` Thomas Abraham
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox