Hi Chandni,
Please find my feedback inline marked [SAMI].
With those fixed,
Reviewed-by: Sami Mujawar <sami.muja...@arm.com>
Regards,
Sami Mujawar
On 04/12/2021 12:30 PM, Chandni Cherukuri wrote:
From: sah01 <sa...@arm.com>
Support has been added to parse NT_FW_CONFIG DTB to get the
platform information.
Signed-off-by: sahil <sa...@arm.com>
Signed-off-by: Chandni Cherukuri <chandni.cheruk...@arm.com>
---
Platform/ARM/Morello/MorelloPlatform.dec | 5 +
Platform/ARM/Morello/Library/PlatformLib/PlatformLibFvp.inf | 6 ++
Platform/ARM/Morello/Library/PlatformLib/PlatformLibSoc.inf | 6 ++
Platform/ARM/Morello/Include/MorelloPlatform.h | 18 +++-
Platform/ARM/Morello/Library/PlatformLib/PlatformLib.c | 9 ++
Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemFvp.c | 74
++++++++++++-
Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemSoc.c | 113
+++++++++++++++++++-
Platform/ARM/Morello/Library/PlatformLib/AArch64/Helper.S | 2 +
8 files changed, 225 insertions(+), 8 deletions(-)
diff --git a/Platform/ARM/Morello/MorelloPlatform.dec
b/Platform/ARM/Morello/MorelloPlatform.dec
index 07701e7611b8..3aec23b03b00 100644
--- a/Platform/ARM/Morello/MorelloPlatform.dec
+++ b/Platform/ARM/Morello/MorelloPlatform.dec
@@ -22,6 +22,8 @@
Include # Root include for the package
[Guids.common]
+ # ARM Morello Platform Info descriptor
+ gArmMorelloPlatformInfoDescriptorGuid = { 0x891422EF, 0x5377, 0x459E, {
0xA5, 0x42, 0x7A, 0xA2, 0x1A, 0x55, 0x54, 0x7F } }
gArmMorelloTokenSpaceGuid = { 0x0A8C3A78, 0xA56F, 0x4788, { 0x83, 0xB4,
0xCD, 0x29, 0x62, 0x96, 0x77, 0x51 } }
[PcdsFixedAtBuild]
@@ -82,3 +84,6 @@
gArmMorelloTokenSpaceGuid.PcdRamDiskSupported|FALSE|BOOLEAN|0x00000007
gArmMorelloTokenSpaceGuid.PcdVirtioBlkSupported|FALSE|BOOLEAN|0x00000008
gArmMorelloTokenSpaceGuid.PcdVirtioNetSupported|FALSE|BOOLEAN|0x0000002C
+
+[Ppis]
+ gNtFwConfigDtInfoPpiGuid = { 0x8E289A83, 0x44E1, 0x41CF, { 0xA7, 0x41,
0x83, 0x80, 0x89, 0x23, 0x43, 0xA3 } }
diff --git a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibFvp.inf
b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibFvp.inf
index d4c8744c0954..0f87a18da184 100644
--- a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibFvp.inf
+++ b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibFvp.inf
@@ -18,10 +18,14 @@
[Packages]
ArmPkg/ArmPkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
Platform/ARM/Morello/MorelloPlatform.dec
+[LibraryClasses]
+ FdtLib
+
[Sources.common]
PlatformLib.c
PlatformLibMemFvp.c
@@ -46,7 +50,9 @@
gArmTokenSpaceGuid.PcdSystemMemorySize
[Guids]
+ gArmMorelloPlatformInfoDescriptorGuid
gEfiHobListGuid ## CONSUMES ## SystemTable
[Ppis]
gArmMpCoreInfoPpiGuid
+ gNtFwConfigDtInfoPpiGuid
diff --git a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibSoc.inf
b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibSoc.inf
index 0a36a5fe50a5..cc0687ebcaaa 100644
--- a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibSoc.inf
+++ b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibSoc.inf
@@ -18,10 +18,14 @@
[Packages]
ArmPkg/ArmPkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
Platform/ARM/Morello/MorelloPlatform.dec
+[LibraryClasses]
+ FdtLib
+
[Sources.common]
PlatformLib.c
PlatformLibMemSoc.c
@@ -56,7 +60,9 @@
gArmTokenSpaceGuid.PcdSystemMemorySize
[Guids]
+ gArmMorelloPlatformInfoDescriptorGuid
gEfiHobListGuid ## CONSUMES ## SystemTable
[Ppis]
gArmMpCoreInfoPpiGuid
+ gNtFwConfigDtInfoPpiGuid
diff --git a/Platform/ARM/Morello/Include/MorelloPlatform.h
b/Platform/ARM/Morello/Include/MorelloPlatform.h
index 8b3233025958..c55bb04445cb 100644
--- a/Platform/ARM/Morello/Include/MorelloPlatform.h
+++ b/Platform/ARM/Morello/Include/MorelloPlatform.h
@@ -51,13 +51,23 @@
*/
#pragma pack(1)
+typedef struct {
+ UINT64 LocalDdrSize; ///< Local DDR memory size in Bytes
+ UINT64 RemoteDdrSize; ///< Remote DDR memory size in Bytes
+ UINT8 RemoteChipCount; ///< Remote chip count in C2C mode
+ UINT8 Mode; ///< 0 - Single Chip, 1 - Chip to Chip (C2C)
+ UINT32 SccConfig; ///< Contains SCC configuration from BOOT_GPR1
register
+} MORELLO_PLAT_INFO_SOC;
+
typedef struct {
UINT64 LocalDdrSize; ///< Local DDR memory size in Bytes
- UINT64 RemoteDdrSize; ///< Remote DDR memory size in Bytes
- UINT8 SlaveCount; ///< Slave count in C2C mode
- UINT8 Mode; ///< 0 - Single Chip, 1 - Chip to Chip (C2C)
-} MORELLO_PLAT_INFO;
+} MORELLO_PLAT_INFO_FVP;
#pragma pack()
+// NT_FW_CONFIG DT structure
+typedef struct {
+ UINT64 NtFwConfigDtAddr;
+} MORELLO_NT_FW_CONFIG_INFO_PPI;
+
#endif //MORELLO_PLATFORM_H_
diff --git a/Platform/ARM/Morello/Library/PlatformLib/PlatformLib.c
b/Platform/ARM/Morello/Library/PlatformLib/PlatformLib.c
index 52318a62911a..b46b3fcc2a7b 100644
--- a/Platform/ARM/Morello/Library/PlatformLib/PlatformLib.c
+++ b/Platform/ARM/Morello/Library/PlatformLib/PlatformLib.c
@@ -8,7 +8,10 @@
#include <Library/ArmPlatformLib.h>
#include <Library/BaseLib.h>
#include <Ppi/ArmMpCoreInfo.h>
+#include <MorelloPlatform.h>
+UINT64 NtFwConfigDtBlob;
+STATIC MORELLO_NT_FW_CONFIG_INFO_PPI mNtFwConfigDtInfoPpi;
STATIC ARM_CORE_INFO mCoreInfoTable[] = {
{ 0x0, 0x0 }, // Cluster 0, Core 0
{ 0x0, 0x1 }, // Cluster 0, Core 1
@@ -44,6 +47,7 @@ ArmPlatformInitialize (
IN UINTN MpId
)
{
+ mNtFwConfigDtInfoPpi.NtFwConfigDtAddr = NtFwConfigDtBlob;
return RETURN_SUCCESS;
}
@@ -76,6 +80,11 @@ STATIC EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
EFI_PEI_PPI_DESCRIPTOR_PPI,
&gArmMpCoreInfoPpiGuid,
&mMpCoreInfoPpi
+ },
+ {
+ EFI_PEI_PPI_DESCRIPTOR_PPI,
+ &gNtFwConfigDtInfoPpiGuid,
+ &mNtFwConfigDtInfoPpi
}
};
diff --git a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemFvp.c b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemFvp.c
index 54a870cfb3ba..9c1da9a25fcd 100644
--- a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemFvp.c
+++ b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemFvp.c
@@ -9,6 +9,8 @@
#include <Library/DebugLib.h>
#include <Library/HobLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/PeiServicesLib.h>
+#include <libfdt.h>
#include <MorelloPlatform.h>
// The total number of descriptors, including the final "end-of-table" descriptor.
@@ -39,6 +41,60 @@ STATIC CONST CHAR8 *gTblAttrDesc[] = {
gTblAttrDesc[VirtualMemoryTable[Index].Attributes] \
));
+/** A helper function to locate the NtFwConfig PPI and get the base address of
+ NT_FW_CONFIG DT from which values are obtained using FDT helper functions.
+
+ @param [out] plat_info Pointer to the MORELLO PLATFORM_INFO HOB
+
+ @retval EFI_SUCCESS Success.
+ returns EFI_INVALID_PARAMETER A parameter is invalid.
+**/
+EFI_STATUS
+GetMorelloPlatInfo (
+ OUT MORELLO_PLAT_INFO_FVP *plat_info
[SAMI] Please rename variable according to edk2 coding standard.
+)
+{
+ CONST UINT64 *Property;
+ INT32 Offset;
+ CONST VOID *NtFwCfgDtBlob;
+ MORELLO_NT_FW_CONFIG_INFO_PPI *NtFwConfigInfoPpi;
+ EFI_STATUS Status;
+
+ Status = PeiServicesLocatePpi (
+ &gNtFwConfigDtInfoPpiGuid,
+ 0,
+ NULL,
+ (VOID**)&NtFwConfigInfoPpi
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR,
+ "PeiServicesLocatePpi failed with error %r\n", Status));
[SAMI] In the current scenario the status code traced and returned do
not match. Can the status code returned by PeiServicesLocatePpi() be
returned here? [/SAMI]
+ return EFI_INVALID_PARAMETER;
+ }
+
+ 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 (NtFwCfgDtBlob, 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 (NtFwCfgDtBlob, Offset, "local-ddr-size", NULL);
+ if (Property == NULL) {
+ DEBUG ((DEBUG_ERROR, "local-ddr-size property not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ plat_info->LocalDdrSize = fdt64_to_cpu (ReadUnaligned64 (Property));
+ return EFI_SUCCESS;
+}
+
/**
Returns the Virtual Memory Map of the platform.
@@ -57,13 +113,27 @@ ArmPlatformGetVirtualMemoryMap (
UINTN Index;
ARM_MEMORY_REGION_DESCRIPTOR * VirtualMemoryTable;
EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
- MORELLO_PLAT_INFO * PlatInfo;
+ MORELLO_PLAT_INFO_FVP * PlatInfo;
UINT64 DramBlock2Size;
+ EFI_STATUS Status;
Index = 0;
DramBlock2Size = 0;
- PlatInfo = (MORELLO_PLAT_INFO *)MORELLO_PLAT_INFO_STRUCT_BASE;
+ // Create platform info HOB
+ PlatInfo = (MORELLO_PLAT_INFO_FVP *)BuildGuidHob (
+ &gArmMorelloPlatformInfoDescriptorGuid,
+ sizeof (MORELLO_PLAT_INFO_FVP)
+ );
+ if (PlatInfo == NULL) {
+ DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
+ ASSERT (FALSE);
+ return;
+ }
+
+ Status = GetMorelloPlatInfo (PlatInfo);
+ ASSERT (Status == 0);
[SAMI] The assert should be 'ASSERT_EFI_STATUS (Status)'.
More importantly the code should not progress if there is
a failure.
Think about what happens for release builds. I think the
code should return in case of failure.
[/SAMI]
+
if (PlatInfo->LocalDdrSize > MORELLO_DRAM_BLOCK1_SIZE) {
DramBlock2Size = PlatInfo->LocalDdrSize - MORELLO_DRAM_BLOCK1_SIZE;
}
diff --git a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemSoc.c
b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemSoc.c
index 5140764c54bc..49e4994176a9 100644
--- a/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemSoc.c
+++ b/Platform/ARM/Morello/Library/PlatformLib/PlatformLibMemSoc.c
@@ -9,6 +9,8 @@
#include <Library/DebugLib.h>
#include <Library/HobLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/PeiServicesLib.h>
+#include <libfdt.h>
#include <MorelloPlatform.h>
// The total number of descriptors, including the final "end-of-table" descriptor.
@@ -39,6 +41,97 @@
gTblAttrDesc[VirtualMemoryTable[Index].Attributes] \
));
+/** A helper function to locate the NtFwConfig PPI and get the base address of
+ NT_FW_CONFIG DT from which values are obtained using FDT helper functions.
+
+ @param [out] plat_info Pointer to the MORELLO PLATFORM_INFO HOB
+
+ @retval EFI_SUCCESS Success.
+ returns EFI_INVALID_PARAMETER A parameter is invalid.
+**/
+EFI_STATUS
+GetMorelloPlatInfo (
+ OUT MORELLO_PLAT_INFO_SOC *plat_info
[SAMI] Please rename variable according to edk2 coding standard.
+ )
+{
+ CONST UINT32 *Property;
+ CONST UINT64 *DdrProperty;
+ INT32 Offset;
+ CONST VOID *NtFwCfgDtBlob;
+ MORELLO_NT_FW_CONFIG_INFO_PPI *NtFwConfigInfoPpi;
+ EFI_STATUS Status;
+
+ 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;
[SAMI] In the current scenario the status code traced and returned do
not match. Can the status code returned by PeiServicesLocatePpi() be
returned here? [/SAMI]
+ }
+
+ 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 (NtFwCfgDtBlob, 0, "platform-info");
+ if (Offset == -FDT_ERR_NOTFOUND) {
+ DEBUG ((DEBUG_ERROR, "Invalid DTB : platform-info node not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DdrProperty = fdt_getprop (NtFwCfgDtBlob, Offset, "local-ddr-size", NULL);
+ if (DdrProperty == NULL) {
+ DEBUG ((DEBUG_ERROR, "local-ddr-size property not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ plat_info->LocalDdrSize = fdt64_to_cpu (ReadUnaligned64 (DdrProperty));
+
+ DdrProperty = fdt_getprop (NtFwCfgDtBlob, Offset, "remote-ddr-size", NULL);
+ if (DdrProperty == NULL) {
+ DEBUG ((DEBUG_ERROR, "remote-ddr-size property not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ plat_info->RemoteDdrSize = fdt64_to_cpu (ReadUnaligned64 (DdrProperty));
+
+ Property = fdt_getprop (NtFwCfgDtBlob, Offset, "remote-chip-count", NULL);
+ if (Property == NULL) {
+ DEBUG ((DEBUG_ERROR, "remote-chip-count property not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ plat_info->RemoteChipCount = fdt32_to_cpu (*Property);
+
+ Property = fdt_getprop (NtFwCfgDtBlob, Offset, "multichip-mode", NULL);
+ if (Property == NULL) {
+ DEBUG ((DEBUG_ERROR, "multichip-mode property not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ plat_info->Mode = fdt32_to_cpu (*Property);
+
+ Property = fdt_getprop (NtFwCfgDtBlob, Offset, "scc-config", NULL);
+ if (Property == NULL) {
+ DEBUG ((DEBUG_ERROR, "scc-config property not found\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ plat_info->SccConfig = fdt32_to_cpu (*Property);
+
+ return EFI_SUCCESS;
+}
+
/**
Returns the Virtual Memory Map of the platform.
@@ -57,17 +150,33 @@ ArmPlatformGetVirtualMemoryMap (
UINTN Index;
ARM_MEMORY_REGION_DESCRIPTOR *VirtualMemoryTable;
EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
- MORELLO_PLAT_INFO *PlatInfo;
+ MORELLO_PLAT_INFO_SOC *PlatInfo;
UINT64 DramBlock2Size;
+ EFI_STATUS Status;
Index = 0;
DramBlock2Size = 0;
- PlatInfo = (MORELLO_PLAT_INFO *)MORELLO_PLAT_INFO_STRUCT_BASE;
+ // Create platform info HOB
+ PlatInfo = (MORELLO_PLAT_INFO_SOC *)BuildGuidHob (
+ &gArmMorelloPlatformInfoDescriptorGuid,
+ sizeof (MORELLO_PLAT_INFO_SOC)
+ );
+
+ if (PlatInfo == NULL) {
+ DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
+ ASSERT (FALSE);
+ return;
+ }
+
+ Status = GetMorelloPlatInfo (PlatInfo);
+ ASSERT (Status == 0);
[SAMI] The assert should be 'ASSERT_EFI_STATUS (Status)'.
More importantly the code should not progress if there is
a failure.
Think about what happens for release builds. I think the
code should return in case of failure.
[/SAMI]
+
if (PlatInfo->LocalDdrSize > MORELLO_DRAM_BLOCK1_SIZE) {
DramBlock2Size = PlatInfo->LocalDdrSize - MORELLO_DRAM_BLOCK1_SIZE;
}
+ DEBUG ((DEBUG_ERROR, "DramBlock2Size is 0x%lx\n", DramBlock2Size));
if (DramBlock2Size != 0) {
ResourceAttributes =
EFI_RESOURCE_ATTRIBUTE_PRESENT |
diff --git a/Platform/ARM/Morello/Library/PlatformLib/AArch64/Helper.S
b/Platform/ARM/Morello/Library/PlatformLib/AArch64/Helper.S
index 0bc624dfd2b4..d71cab916c75 100644
--- a/Platform/ARM/Morello/Library/PlatformLib/AArch64/Helper.S
+++ b/Platform/ARM/Morello/Library/PlatformLib/AArch64/Helper.S
@@ -19,6 +19,8 @@
// the UEFI firmware through the CPU registers.
//
ASM_FUNC(ArmPlatformPeiBootAction)
+ adr x10, NtFwConfigDtBlob
+ str x0, [x10]
ret
//
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84472): https://edk2.groups.io/g/devel/message/84472
Mute This Topic: https://groups.io/mt/87497028/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
