* [PATCH 0/2] ARM Dynamic Configuration
@ 2018-06-15 5:55 Chandni Cherukuri
2018-06-15 5:55 ` [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB Chandni Cherukuri
2018-06-15 5:55 ` [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID Chandni Cherukuri
0 siblings, 2 replies; 10+ messages in thread
From: Chandni Cherukuri @ 2018-06-15 5:55 UTC (permalink / raw)
To: edk2-devel; +Cc: ard.biesheuvel, leif.lindholm
On SGI platforms, the trusted firmware executes prior to the SEC
phase. It supplies to the SEC phase a pointer to a HW CONFIG fdt
in the x1 which contains platform specific information such as part
number and config number of the SGI platform.
The HW CONFIG FDT would look as,
/dts-v1/;
/ {
/* compatible string */
compatible = "arm,sgi-isys3";
/* platform ID node */
system-id {
platform-id = <0x03000783>;
}
};
In the very first step of the assembly code which executes in SEC phase
the fdt pointer is stored in a global variable from the register. A PPI
is created during the SEC phase which stores the global variable.
During PEI phase, a Platform ID PEIM is installed which accessess the PPI
and retrieves the platform information using FDT helper functions and a
PlatformId HOB is created and populated with the information.
During DXE phase the drivers can access the Platform ID HOB to get the
platform information and perform platform specific functions based on the
platform.
Chandni Cherukuri (2):
Platform/ARM/Sgi: Install a Platform ID HOB
Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID
.../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c | 25 +++++-
.../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 2 +
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 18 +++++
.../SgiPkg/Library/PlatformLib/AArch64/Helper.S | 7 ++
.../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 +++
.../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +
.../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c | 92 ++++++++++++++++++++++
.../SgiPlatformPeiLib/SgiPlatformPeiLib.inf | 40 ++++++++++
Platform/ARM/SgiPkg/SgiPlatform.dec | 5 ++
Platform/ARM/SgiPkg/SgiPlatform.dsc | 1 +
Platform/ARM/SgiPkg/SgiPlatform.fdf | 1 +
11 files changed, 203 insertions(+), 1 deletion(-)
create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB
2018-06-15 5:55 [PATCH 0/2] ARM Dynamic Configuration Chandni Cherukuri
@ 2018-06-15 5:55 ` Chandni Cherukuri
2018-06-15 6:16 ` Ard Biesheuvel
2018-06-15 10:18 ` Leif Lindholm
2018-06-15 5:55 ` [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID Chandni Cherukuri
1 sibling, 2 replies; 10+ messages in thread
From: Chandni Cherukuri @ 2018-06-15 5:55 UTC (permalink / raw)
To: edk2-devel; +Cc: ard.biesheuvel, leif.lindholm
On SGI platforms, the trusted firmware executes prior to
the SEC phase. It supplies to the SEC phase a pointer to
a fdt, that contains platform specific information such as
part number and config number of the SGI platform. The
platform code that executes during the SEC phase creates a
PPI that allows access to other PEIMs to obtain a copy of the
fdt pointer.
Further, during the PEI phase, a Platform ID PEIM installs a
Platform ID HOB. The Platform ID HOB can be used by DXE
drivers to identify the platform it is executing on.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
---
.../SgiPkg/Library/PlatformLib/AArch64/Helper.S | 15 ++-
.../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 ++
.../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +
.../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c | 108 +++++++++++++++++++++
.../SgiPlatformPeiLib/SgiPlatformPeiLib.inf | 40 ++++++++
Platform/ARM/SgiPkg/SgiPlatform.dec | 5 +
6 files changed, 177 insertions(+), 4 deletions(-)
create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
index dab6c77..80b93ea 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
@@ -22,15 +22,22 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
+GCC_ASM_EXPORT(HwConfigDtBlob)
+
+HwConfigDtBlob: .long 0
//
// First platform specific function to be called in the PEI phase
//
-// This function is actually the first function called by the PrePi
-// or PrePeiCore modules. It allows to retrieve arguments passed to
-// the UEFI firmware through the CPU registers.
-//
+// The trusted firmware passed the hw config DT blob in x1 register.
+// Keep a copy of it in a global variable.
+//VOID
+//ArmPlatformPeiBootAction (
+// VOID
+// );
ASM_PFX(ArmPlatformPeiBootAction):
+ ldr x10, =HwConfigDtBlob
+ str x1, [x10]
ret
//UINTN
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
index ea3201a..54860e0 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
@@ -15,6 +15,10 @@
#include <Library/ArmPlatformLib.h>
#include <Library/BaseLib.h>
#include <Ppi/ArmMpCoreInfo.h>
+#include <SgiPlatform.h>
+
+extern UINT64 HwConfigDtBlob;
+STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
STATIC ARM_CORE_INFO mCoreInfoTable[] = {
{
@@ -36,6 +40,7 @@ ArmPlatformInitialize (
IN UINTN MpId
)
{
+ mHwConfigDtInfoPpi.HwConfigDtAddr = &HwConfigDtBlob;
return RETURN_SUCCESS;
}
@@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
EFI_PEI_PPI_DESCRIPTOR_PPI,
&gArmMpCoreInfoPpiGuid,
&mMpCoreInfoPpi
+ },
+ {
+ EFI_PEI_PPI_DESCRIPTOR_PPI,
+ &gHwConfigDtInfoPpiGuid,
+ &mHwConfigDtInfoPpi
}
};
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
index 42e14d5..5d4bf72 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
@@ -61,7 +61,10 @@
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
[Guids]
+ gArmSgiPlatformIdDescriptorGuid
gEfiHobListGuid ## CONSUMES ## SystemTable
+ gFdtTableGuid
[Ppis]
gArmMpCoreInfoPpiGuid
+ gHwConfigDtInfoPpiGuid
diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
new file mode 100644
index 0000000..f6446e6
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
@@ -0,0 +1,108 @@
+/** @file
+*
+* Copyright (c) 2018, ARM Limited. All rights reserved.
+*
+* This program and the accompanying materials are licensed and made available
+* under the terms and conditions of the BSD License which accompanies this
+* distribution. The full text of the license may be found at
+* http://opensource.org/licenses/bsd-license.php
+*
+* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PeimEntryPoint.h>
+#include <Library/PeiServicesLib.h>
+#include <libfdt.h>
+#include <SgiPlatform.h>
+
+/**
+
+ This function returns the Platform ID of the platform
+
+ This functions locates the HwConfig PPI and gets the base address of HW CONFIG
+ DT from which the platform ID is obtained using FDT helper functions
+
+ @return returns the platform ID on success else returns 0 on error
+
+**/
+
+STATIC
+UINT32
+GetSgiPlatformId (
+ VOID
+)
+{
+ CONST UINT32 *Property;
+ INT32 Offset;
+ CONST VOID *HwCfgDtBlob;
+ SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi;
+ EFI_STATUS Status;
+
+ Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, (VOID**)&HwConfigInfoPpi);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "PeiServicesLocatePpi failed with error %r\n", Status));
+ return 0;
+ }
+
+ HwCfgDtBlob = (VOID *)(*(HwConfigInfoPpi->HwConfigDtAddr));
+ if (fdt_check_header (HwCfgDtBlob) != 0 ) {
+ DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed as HW_CONFIG\n", HwCfgDtBlob));
+ return 0;
+ }
+
+ Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id");
+ if (Offset == -FDT_ERR_NOTFOUND) {
+ DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n"));
+ return 0;
+ }
+
+ Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
+ if (Property == NULL) {
+ DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
+ return 0;
+ }
+
+ return fdt32_to_cpu (*Property);
+}
+
+/**
+
+ This function creates a Platform ID HOB and assigns PlatformId as the
+ HobData
+
+ @return asserts on error and returns EFI_INVALID_PARAMETER. On success
+ returns EFI_SUCCESS
+
+**/
+EFI_STATUS
+EFIAPI
+SgiPlatformIdPeim (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+)
+{
+ SGI_PLATFORM_DESCRIPTOR *HobData;
+
+ //Create platform descriptor HOB
+ HobData = (SGI_PLATFORM_DESCRIPTOR *)BuildGuidHob (&gArmSgiPlatformIdDescriptorGuid,
+ sizeof (SGI_PLATFORM_DESCRIPTOR));
+
+ //Get the platform id from the platform specific hw_config device tree
+ if (HobData != NULL) {
+ HobData->PlatformId = GetSgiPlatformId ();
+ if (HobData->PlatformId == 0) {
+ ASSERT (FALSE);
+ return EFI_INVALID_PARAMETER;
+ }
+ } else {
+ DEBUG ((DEBUG_ERROR, "Platform HoB is NULL\n"));
+ ASSERT (FALSE);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
new file mode 100644
index 0000000..502d6ac
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
@@ -0,0 +1,40 @@
+#
+# Copyright (c) 2018, ARM Limited. All rights reserved.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = SgiPlatformIdPeiLib
+ FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ ENTRY_POINT = SgiPlatformIdPeim
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+ Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+ FdtLib
+ PeimEntryPoint
+
+[Sources]
+ SgiPlatformPeiLib.c
+
+[Guids]
+ gArmSgiPlatformIdDescriptorGuid
+
+[Ppis]
+ gHwConfigDtInfoPpiGuid
+
+[Depex]
+ TRUE
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
index d995937..ea9f6c5 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -29,9 +29,14 @@
Include # Root include for the package
[Guids.common]
+ # ARM Sgi Platform ID descriptor
+ gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } }
gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } }
[PcdsFeatureFlag.common]
# Set this PCD to TRUE to enable virtio support.
gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001
+
+[Ppis]
+ gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID
2018-06-15 5:55 [PATCH 0/2] ARM Dynamic Configuration Chandni Cherukuri
2018-06-15 5:55 ` [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB Chandni Cherukuri
@ 2018-06-15 5:55 ` Chandni Cherukuri
2018-06-15 6:18 ` Ard Biesheuvel
2018-06-15 10:22 ` Leif Lindholm
1 sibling, 2 replies; 10+ messages in thread
From: Chandni Cherukuri @ 2018-06-15 5:55 UTC (permalink / raw)
To: edk2-devel; +Cc: ard.biesheuvel, leif.lindholm
Use the platform ID and config ID values from the platform ID
HOB to choose the right set of ACPI tables to be installed.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
---
.../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c | 33 ++++++++++++++++++----
.../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 2 ++
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 24 ++++++++++++++--
Platform/ARM/SgiPkg/SgiPlatform.dsc | 1 +
Platform/ARM/SgiPkg/SgiPlatform.fdf | 1 +
5 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
index edaae5b..d0d5808 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -14,6 +14,8 @@
#include <Library/AcpiLib.h>
#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <SgiPlatform.h>
EFI_STATUS
InitVirtioBlockIo (
@@ -28,20 +30,41 @@ ArmSgiPkgEntryPoint (
)
{
EFI_STATUS Status;
+ VOID *PlatformIdHob;
+ SGI_PLATFORM_DESCRIPTOR *HobData;
+ UINT32 ConfigId;
+ UINT32 PartNum;
- Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesiFileGuid);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
- return Status;
+ PlatformIdHob = GetFirstGuidHob (&gArmSgiPlatformIdDescriptorGuid);
+ if (PlatformIdHob == NULL) {
+ DEBUG ((DEBUG_ERROR, "Platform ID Hob is NULL\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ HobData = (SGI_PLATFORM_DESCRIPTOR *)(GET_GUID_HOB_DATA (PlatformIdHob));
+
+ PartNum = (HobData->PlatformId & SGI_PART_NUM_MASK);
+ ConfigId = ((HobData->PlatformId >> SGI_CONFIG_SHIFT) & SGI_CONFIG_MASK);
+
+ if ((PartNum == SGI575_PART_NUM) && (ConfigId == SGI575_CONF_NUM)) {
+ Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesiFileGuid);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
+ return Status;
+ }
+ } else {
+ DEBUG ((DEBUG_ERROR, "PlatformDxe: Unsupported Platform Id\n"));
+ return EFI_UNSUPPORTED;
}
- Status = EFI_REQUEST_UNLOAD_IMAGE;
if (FeaturePcdGet (PcdVirtioSupported)) {
Status = InitVirtioBlockIo (ImageHandle);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "%a: Failed to install Virtio Block device\n",
__FUNCTION__));
}
+ } else {
+ Status = EFI_REQUEST_UNLOAD_IMAGE;
}
return Status;
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
index 51ad22f..b6b8209 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -30,10 +30,12 @@
[LibraryClasses]
AcpiLib
+ HobLib
UefiDriverEntryPoint
VirtioMmioDeviceLib
[Guids]
+ gArmSgiPlatformIdDescriptorGuid
gSgi575AcpiTablesiFileGuid
[FeaturePcd]
diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index 00ca7e9..756954c 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -12,8 +12,8 @@
*
**/
-#ifndef __SGI_PLATFORM_H__
-#define __SGI_PLATFORM_H__
+#ifndef SGI_PLATFORM_H_
+#define SGI_PLATFORM_H_
/***********************************************************************************
// Platform Memory Map
@@ -68,4 +68,22 @@
#define SGI_SYSPH_SYS_REG_FLASH 0x4C
#define SGI_SYSPH_SYS_REG_FLASH_RWEN 0x1
-#endif // __SGI_PLATFORM_H__
+// SGI575_VERSION values
+#define SGI575_CONF_NUM 0x3
+#define SGI575_PART_NUM 0x783
+
+#define SGI_CONFIG_MASK 0x0F
+#define SGI_CONFIG_SHIFT 0x1C
+#define SGI_PART_NUM_MASK 0xFFF
+
+//HwConfig DT structure^M
+typedef struct {
+ UINT64 *HwConfigDtAddr;
+} SGI_HW_CONFIG_INFO_PPI;
+
+// ARM platform description data.
+typedef struct {
+ UINTN PlatformId;
+} SGI_PLATFORM_DESCRIPTOR;
+
+#endif // SGI_PLATFORM_H_
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
index a56175e..f571897 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
@@ -199,6 +199,7 @@
<LibraryClasses>
NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
}
+ Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
#
# DXE
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
index 17cdf48..05203a8 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
+++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
@@ -220,6 +220,7 @@ READ_LOCK_STATUS = TRUE
INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+ INF Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB
2018-06-15 5:55 ` [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB Chandni Cherukuri
@ 2018-06-15 6:16 ` Ard Biesheuvel
2018-06-15 6:19 ` Ard Biesheuvel
2018-06-15 10:18 ` Leif Lindholm
1 sibling, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-15 6:16 UTC (permalink / raw)
To: Chandni Cherukuri; +Cc: edk2-devel@lists.01.org, Leif Lindholm
Hello Chandni,
On 15 June 2018 at 07:55, Chandni Cherukuri <chandni.cherukuri@arm.com> wrote:
> On SGI platforms, the trusted firmware executes prior to
> the SEC phase. It supplies to the SEC phase a pointer to
> a fdt, that contains platform specific information such as
> part number and config number of the SGI platform. The
> platform code that executes during the SEC phase creates a
> PPI that allows access to other PEIMs to obtain a copy of the
> fdt pointer.
>
> Further, during the PEI phase, a Platform ID PEIM installs a
> Platform ID HOB. The Platform ID HOB can be used by DXE
> drivers to identify the platform it is executing on.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> ---
> .../SgiPkg/Library/PlatformLib/AArch64/Helper.S | 15 ++-
> .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 ++
> .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +
> .../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c | 108 +++++++++++++++++++++
> .../SgiPlatformPeiLib/SgiPlatformPeiLib.inf | 40 ++++++++
> Platform/ARM/SgiPkg/SgiPlatform.dec | 5 +
> 6 files changed, 177 insertions(+), 4 deletions(-)
> create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>
Please use '--stat=250 --stat-graph-width=20' when using git
format-patch so that we get to see the entire path names in the
diffstat
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> index dab6c77..80b93ea 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> @@ -22,15 +22,22 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> +GCC_ASM_EXPORT(HwConfigDtBlob)
> +
> +HwConfigDtBlob: .long 0
>
Please move this definition to PlatformLib.c
> //
> // First platform specific function to be called in the PEI phase
> //
> -// This function is actually the first function called by the PrePi
> -// or PrePeiCore modules. It allows to retrieve arguments passed to
> -// the UEFI firmware through the CPU registers.
> -//
> +// The trusted firmware passed the hw config DT blob in x1 register.
> +// Keep a copy of it in a global variable.
> +//VOID
> +//ArmPlatformPeiBootAction (
> +// VOID
> +// );
> ASM_PFX(ArmPlatformPeiBootAction):
> + ldr x10, =HwConfigDtBlob
Please use adr to take the address of HwConfigDtBlob
> + str x1, [x10]
> ret
>
> //UINTN
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> index ea3201a..54860e0 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> @@ -15,6 +15,10 @@
> #include <Library/ArmPlatformLib.h>
> #include <Library/BaseLib.h>
> #include <Ppi/ArmMpCoreInfo.h>
> +#include <SgiPlatform.h>
> +
> +extern UINT64 HwConfigDtBlob;
> +STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
>
Where is this PPI declared? A PPI is like a protocol, it has its own
declaration in a header file under Include/Ppi in the package
> STATIC ARM_CORE_INFO mCoreInfoTable[] = {
> {
> @@ -36,6 +40,7 @@ ArmPlatformInitialize (
> IN UINTN MpId
> )
> {
> + mHwConfigDtInfoPpi.HwConfigDtAddr = &HwConfigDtBlob;
> return RETURN_SUCCESS;
> }
>
> @@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
> EFI_PEI_PPI_DESCRIPTOR_PPI,
> &gArmMpCoreInfoPpiGuid,
> &mMpCoreInfoPpi
> + },
> + {
> + EFI_PEI_PPI_DESCRIPTOR_PPI,
> + &gHwConfigDtInfoPpiGuid,
> + &mHwConfigDtInfoPpi
> }
> };
>
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> index 42e14d5..5d4bf72 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> @@ -61,7 +61,10 @@
> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
> [Guids]
> + gArmSgiPlatformIdDescriptorGuid
> gEfiHobListGuid ## CONSUMES ## SystemTable
> + gFdtTableGuid
>
> [Ppis]
> gArmMpCoreInfoPpiGuid
> + gHwConfigDtInfoPpiGuid
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> new file mode 100644
> index 0000000..f6446e6
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> @@ -0,0 +1,108 @@
> +/** @file
> +*
> +* Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +* This program and the accompanying materials are licensed and made available
> +* under the terms and conditions of the BSD License which accompanies this
> +* distribution. The full text of the license may be found at
> +* http://opensource.org/licenses/bsd-license.php
> +*
> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PeimEntryPoint.h>
> +#include <Library/PeiServicesLib.h>
> +#include <libfdt.h>
> +#include <SgiPlatform.h>
> +
> +/**
> +
> + This function returns the Platform ID of the platform
> +
> + This functions locates the HwConfig PPI and gets the base address of HW CONFIG
> + DT from which the platform ID is obtained using FDT helper functions
> +
> + @return returns the platform ID on success else returns 0 on error
> +
> +**/
> +
> +STATIC
> +UINT32
> +GetSgiPlatformId (
> + VOID
> +)
> +{
> + CONST UINT32 *Property;
> + INT32 Offset;
> + CONST VOID *HwCfgDtBlob;
> + SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi;
> + EFI_STATUS Status;
> +
> + Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, (VOID**)&HwConfigInfoPpi);
Please check all your patches for overly long lines. 80 columns is that maximum
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "PeiServicesLocatePpi failed with error %r\n", Status));
> + return 0;
> + }
> +
> + HwCfgDtBlob = (VOID *)(*(HwConfigInfoPpi->HwConfigDtAddr));
> + if (fdt_check_header (HwCfgDtBlob) != 0 ) {
> + DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed as HW_CONFIG\n", HwCfgDtBlob));
> + return 0;
> + }
> +
> + Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id");
> + if (Offset == -FDT_ERR_NOTFOUND) {
> + DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n"));
> + return 0;
> + }
> +
> + Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
> + if (Property == NULL) {
> + DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
> + return 0;
> + }
> +
> + return fdt32_to_cpu (*Property);
> +}
> +
> +/**
> +
> + This function creates a Platform ID HOB and assigns PlatformId as the
> + HobData
> +
> + @return asserts on error and returns EFI_INVALID_PARAMETER. On success
> + returns EFI_SUCCESS
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SgiPlatformIdPeim (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> +)
> +{
> + SGI_PLATFORM_DESCRIPTOR *HobData;
> +
> + //Create platform descriptor HOB
> + HobData = (SGI_PLATFORM_DESCRIPTOR *)BuildGuidHob (&gArmSgiPlatformIdDescriptorGuid,
> + sizeof (SGI_PLATFORM_DESCRIPTOR));
> +
> + //Get the platform id from the platform specific hw_config device tree
> + if (HobData != NULL) {
Please invert the sense of this conditional, and return
EFI_OUT_OF_RESOURCES if HobData is NULL
> + HobData->PlatformId = GetSgiPlatformId ();
> + if (HobData->PlatformId == 0) {
> + ASSERT (FALSE);
> + return EFI_INVALID_PARAMETER;
> + }
> + } else {
After you have inverted the if() condition, you can drop the else as well
> + DEBUG ((DEBUG_ERROR, "Platform HoB is NULL\n"));
> + ASSERT (FALSE);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> new file mode 100644
> index 0000000..502d6ac
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> @@ -0,0 +1,40 @@
> +#
> +# Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
> + BASE_NAME = SgiPlatformIdPeiLib
> + FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
> + MODULE_TYPE = PEIM
> + VERSION_STRING = 1.0
> + ENTRY_POINT = SgiPlatformIdPeim
> +
> +[Packages]
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdePkg/MdePkg.dec
> + Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[LibraryClasses]
> + FdtLib
> + PeimEntryPoint
> +
> +[Sources]
> + SgiPlatformPeiLib.c
> +
> +[Guids]
> + gArmSgiPlatformIdDescriptorGuid
> +
> +[Ppis]
> + gHwConfigDtInfoPpiGuid
> +
> +[Depex]
> + TRUE
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> index d995937..ea9f6c5 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -29,9 +29,14 @@
> Include # Root include for the package
>
> [Guids.common]
> + # ARM Sgi Platform ID descriptor
> + gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } }
> gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
> gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } }
>
> [PcdsFeatureFlag.common]
> # Set this PCD to TRUE to enable virtio support.
> gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001
> +
> +[Ppis]
> + gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID
2018-06-15 5:55 ` [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID Chandni Cherukuri
@ 2018-06-15 6:18 ` Ard Biesheuvel
2018-06-15 10:22 ` Leif Lindholm
1 sibling, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-15 6:18 UTC (permalink / raw)
To: Chandni Cherukuri; +Cc: edk2-devel@lists.01.org, Leif Lindholm
Hello Chandni,
On 15 June 2018 at 07:55, Chandni Cherukuri <chandni.cherukuri@arm.com> wrote:
> Use the platform ID and config ID values from the platform ID
> HOB to choose the right set of ACPI tables to be installed.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> ---
> .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c | 33 ++++++++++++++++++----
> .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 2 ++
> Platform/ARM/SgiPkg/Include/SgiPlatform.h | 24 ++++++++++++++--
> Platform/ARM/SgiPkg/SgiPlatform.dsc | 1 +
> Platform/ARM/SgiPkg/SgiPlatform.fdf | 1 +
> 5 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index edaae5b..d0d5808 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -14,6 +14,8 @@
>
> #include <Library/AcpiLib.h>
> #include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <SgiPlatform.h>
>
> EFI_STATUS
> InitVirtioBlockIo (
> @@ -28,20 +30,41 @@ ArmSgiPkgEntryPoint (
> )
> {
> EFI_STATUS Status;
> + VOID *PlatformIdHob;
> + SGI_PLATFORM_DESCRIPTOR *HobData;
> + UINT32 ConfigId;
> + UINT32 PartNum;
>
> - Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesiFileGuid);
> - if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
> - return Status;
> + PlatformIdHob = GetFirstGuidHob (&gArmSgiPlatformIdDescriptorGuid);
> + if (PlatformIdHob == NULL) {
> + DEBUG ((DEBUG_ERROR, "Platform ID Hob is NULL\n"));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + HobData = (SGI_PLATFORM_DESCRIPTOR *)(GET_GUID_HOB_DATA (PlatformIdHob));
> +
> + PartNum = (HobData->PlatformId & SGI_PART_NUM_MASK);
> + ConfigId = ((HobData->PlatformId >> SGI_CONFIG_SHIFT) & SGI_CONFIG_MASK);
> +
> + if ((PartNum == SGI575_PART_NUM) && (ConfigId == SGI575_CONF_NUM)) {
> + Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesiFileGuid);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
> + return Status;
> + }
> + } else {
> + DEBUG ((DEBUG_ERROR, "PlatformDxe: Unsupported Platform Id\n"));
> + return EFI_UNSUPPORTED;
> }
>
> - Status = EFI_REQUEST_UNLOAD_IMAGE;
> if (FeaturePcdGet (PcdVirtioSupported)) {
> Status = InitVirtioBlockIo (ImageHandle);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a: Failed to install Virtio Block device\n",
> __FUNCTION__));
> }
> + } else {
> + Status = EFI_REQUEST_UNLOAD_IMAGE;
> }
>
> return Status;
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index 51ad22f..b6b8209 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -30,10 +30,12 @@
>
> [LibraryClasses]
> AcpiLib
> + HobLib
> UefiDriverEntryPoint
> VirtioMmioDeviceLib
>
> [Guids]
> + gArmSgiPlatformIdDescriptorGuid
> gSgi575AcpiTablesiFileGuid
>
> [FeaturePcd]
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> index 00ca7e9..756954c 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -12,8 +12,8 @@
> *
> **/
>
> -#ifndef __SGI_PLATFORM_H__
> -#define __SGI_PLATFORM_H__
> +#ifndef SGI_PLATFORM_H_
> +#define SGI_PLATFORM_H_
>
> /***********************************************************************************
> // Platform Memory Map
> @@ -68,4 +68,22 @@
> #define SGI_SYSPH_SYS_REG_FLASH 0x4C
> #define SGI_SYSPH_SYS_REG_FLASH_RWEN 0x1
>
> -#endif // __SGI_PLATFORM_H__
> +// SGI575_VERSION values
> +#define SGI575_CONF_NUM 0x3
> +#define SGI575_PART_NUM 0x783
> +
> +#define SGI_CONFIG_MASK 0x0F
> +#define SGI_CONFIG_SHIFT 0x1C
> +#define SGI_PART_NUM_MASK 0xFFF
> +
> +//HwConfig DT structure^M
^M ?
> +typedef struct {
> + UINT64 *HwConfigDtAddr;
> +} SGI_HW_CONFIG_INFO_PPI;
> +
Please declare this PPI properly
> +// ARM platform description data.
> +typedef struct {
> + UINTN PlatformId;
> +} SGI_PLATFORM_DESCRIPTOR;
> +
> +#endif // SGI_PLATFORM_H_
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> index a56175e..f571897 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -199,6 +199,7 @@
> <LibraryClasses>
> NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> }
> + Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>
> #
> # DXE
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> index 17cdf48..05203a8 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> @@ -220,6 +220,7 @@ READ_LOCK_STATUS = TRUE
> INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> + INF Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>
> FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB
2018-06-15 6:16 ` Ard Biesheuvel
@ 2018-06-15 6:19 ` Ard Biesheuvel
2018-06-15 9:03 ` Leif Lindholm
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-06-15 6:19 UTC (permalink / raw)
To: Chandni Cherukuri; +Cc: edk2-devel@lists.01.org, Leif Lindholm
On 15 June 2018 at 08:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Hello Chandni,
>
> On 15 June 2018 at 07:55, Chandni Cherukuri <chandni.cherukuri@arm.com> wrote:
>> On SGI platforms, the trusted firmware executes prior to
>> the SEC phase. It supplies to the SEC phase a pointer to
>> a fdt, that contains platform specific information such as
>> part number and config number of the SGI platform. The
>> platform code that executes during the SEC phase creates a
>> PPI that allows access to other PEIMs to obtain a copy of the
>> fdt pointer.
>>
>> Further, during the PEI phase, a Platform ID PEIM installs a
>> Platform ID HOB. The Platform ID HOB can be used by DXE
>> drivers to identify the platform it is executing on.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
>> ---
>> .../SgiPkg/Library/PlatformLib/AArch64/Helper.S | 15 ++-
>> .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 ++
>> .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +
>> .../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c | 108 +++++++++++++++++++++
>> .../SgiPlatformPeiLib/SgiPlatformPeiLib.inf | 40 ++++++++
>> Platform/ARM/SgiPkg/SgiPlatform.dec | 5 +
>> 6 files changed, 177 insertions(+), 4 deletions(-)
>> create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
>> create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>>
>
> Please use '--stat=250 --stat-graph-width=20' when using git
> format-patch so that we get to see the entire path names in the
> diffstat
>
>> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
>> index dab6c77..80b93ea 100644
>> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
>> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
>> @@ -22,15 +22,22 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
>> GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
>> GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
>> GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
>> +GCC_ASM_EXPORT(HwConfigDtBlob)
>> +
>> +HwConfigDtBlob: .long 0
>>
>
> Please move this definition to PlatformLib.c
>
>> //
>> // First platform specific function to be called in the PEI phase
>> //
>> -// This function is actually the first function called by the PrePi
>> -// or PrePeiCore modules. It allows to retrieve arguments passed to
>> -// the UEFI firmware through the CPU registers.
>> -//
>> +// The trusted firmware passed the hw config DT blob in x1 register.
>> +// Keep a copy of it in a global variable.
>> +//VOID
>> +//ArmPlatformPeiBootAction (
>> +// VOID
>> +// );
>> ASM_PFX(ArmPlatformPeiBootAction):
>> + ldr x10, =HwConfigDtBlob
>
> Please use adr to take the address of HwConfigDtBlob
>
>> + str x1, [x10]
>> ret
>>
>> //UINTN
>> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
>> index ea3201a..54860e0 100644
>> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
>> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
>> @@ -15,6 +15,10 @@
>> #include <Library/ArmPlatformLib.h>
>> #include <Library/BaseLib.h>
>> #include <Ppi/ArmMpCoreInfo.h>
>> +#include <SgiPlatform.h>
>> +
>> +extern UINT64 HwConfigDtBlob;
>> +STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
>>
>
> Where is this PPI declared? A PPI is like a protocol, it has its own
> declaration in a header file under Include/Ppi in the package
>
>> STATIC ARM_CORE_INFO mCoreInfoTable[] = {
>> {
>> @@ -36,6 +40,7 @@ ArmPlatformInitialize (
>> IN UINTN MpId
>> )
>> {
>> + mHwConfigDtInfoPpi.HwConfigDtAddr = &HwConfigDtBlob;
>> return RETURN_SUCCESS;
>> }
>>
>> @@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
>> EFI_PEI_PPI_DESCRIPTOR_PPI,
>> &gArmMpCoreInfoPpiGuid,
>> &mMpCoreInfoPpi
>> + },
>> + {
>> + EFI_PEI_PPI_DESCRIPTOR_PPI,
>> + &gHwConfigDtInfoPpiGuid,
>> + &mHwConfigDtInfoPpi
>> }
>> };
>>
>> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
>> index 42e14d5..5d4bf72 100644
>> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
>> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
>> @@ -61,7 +61,10 @@
>> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>>
>> [Guids]
>> + gArmSgiPlatformIdDescriptorGuid
>> gEfiHobListGuid ## CONSUMES ## SystemTable
>> + gFdtTableGuid
>>
>> [Ppis]
>> gArmMpCoreInfoPpiGuid
>> + gHwConfigDtInfoPpiGuid
>> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
>> new file mode 100644
>> index 0000000..f6446e6
>> --- /dev/null
>> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
>> @@ -0,0 +1,108 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2018, ARM Limited. All rights reserved.
>> +*
>> +* This program and the accompanying materials are licensed and made available
>> +* under the terms and conditions of the BSD License which accompanies this
>> +* distribution. The full text of the license may be found at
>> +* http://opensource.org/licenses/bsd-license.php
>> +*
>> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +*
>> +**/
>> +
>> +#include <Library/DebugLib.h>
>> +#include <Library/HobLib.h>
>> +#include <Library/PeimEntryPoint.h>
>> +#include <Library/PeiServicesLib.h>
>> +#include <libfdt.h>
>> +#include <SgiPlatform.h>
>> +
>> +/**
>> +
>> + This function returns the Platform ID of the platform
>> +
>> + This functions locates the HwConfig PPI and gets the base address of HW CONFIG
>> + DT from which the platform ID is obtained using FDT helper functions
>> +
>> + @return returns the platform ID on success else returns 0 on error
>> +
>> +**/
>> +
>> +STATIC
>> +UINT32
>> +GetSgiPlatformId (
>> + VOID
>> +)
>> +{
>> + CONST UINT32 *Property;
>> + INT32 Offset;
>> + CONST VOID *HwCfgDtBlob;
>> + SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi;
>> + EFI_STATUS Status;
>> +
>> + Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, (VOID**)&HwConfigInfoPpi);
>
> Please check all your patches for overly long lines. 80 columns is that maximum
>
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_ERROR, "PeiServicesLocatePpi failed with error %r\n", Status));
>> + return 0;
>> + }
>> +
>> + HwCfgDtBlob = (VOID *)(*(HwConfigInfoPpi->HwConfigDtAddr));
>> + if (fdt_check_header (HwCfgDtBlob) != 0 ) {
>> + DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed as HW_CONFIG\n", HwCfgDtBlob));
>> + return 0;
>> + }
>> +
>> + Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id");
>> + if (Offset == -FDT_ERR_NOTFOUND) {
>> + DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n"));
>> + return 0;
>> + }
>> +
>> + Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
>> + if (Property == NULL) {
>> + DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
>> + return 0;
>> + }
>> +
>> + return fdt32_to_cpu (*Property);
>> +}
>> +
>> +/**
>> +
>> + This function creates a Platform ID HOB and assigns PlatformId as the
>> + HobData
>> +
>> + @return asserts on error and returns EFI_INVALID_PARAMETER. On success
>> + returns EFI_SUCCESS
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SgiPlatformIdPeim (
>> + IN EFI_PEI_FILE_HANDLE FileHandle,
>> + IN CONST EFI_PEI_SERVICES **PeiServices
>> +)
>> +{
>> + SGI_PLATFORM_DESCRIPTOR *HobData;
>> +
>> + //Create platform descriptor HOB
>> + HobData = (SGI_PLATFORM_DESCRIPTOR *)BuildGuidHob (&gArmSgiPlatformIdDescriptorGuid,
>> + sizeof (SGI_PLATFORM_DESCRIPTOR));
>> +
>> + //Get the platform id from the platform specific hw_config device tree
>> + if (HobData != NULL) {
>
> Please invert the sense of this conditional, and return
> EFI_OUT_OF_RESOURCES if HobData is NULL
>
>> + HobData->PlatformId = GetSgiPlatformId ();
>> + if (HobData->PlatformId == 0) {
>> + ASSERT (FALSE);
>> + return EFI_INVALID_PARAMETER;
>> + }
>> + } else {
>
> After you have inverted the if() condition, you can drop the else as well
>
>> + DEBUG ((DEBUG_ERROR, "Platform HoB is NULL\n"));
>> + ASSERT (FALSE);
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>> new file mode 100644
>> index 0000000..502d6ac
>> --- /dev/null
>> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>> @@ -0,0 +1,40 @@
>> +#
>> +# Copyright (c) 2018, ARM Limited. All rights reserved.
>> +#
>> +# This program and the accompanying materials are licensed and made available
>> +# under the terms and conditions of the BSD License which accompanies this
>> +# distribution. The full text of the license may be found at
>> +# http://opensource.org/licenses/bsd-license.php
>> +#
>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +
>> +[Defines]
>> + INF_VERSION = 0x0001001A
>> + BASE_NAME = SgiPlatformIdPeiLib
I forgot: this is a module not a library. So it should be called
SgiPlatformIdPei or SgiPlatformIdPeim not ...Lib
>> + FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
>> + MODULE_TYPE = PEIM
>> + VERSION_STRING = 1.0
>> + ENTRY_POINT = SgiPlatformIdPeim
>> +
>> +[Packages]
>> + EmbeddedPkg/EmbeddedPkg.dec
>> + MdePkg/MdePkg.dec
>> + Platform/ARM/SgiPkg/SgiPlatform.dec
>> +
>> +[LibraryClasses]
>> + FdtLib
>> + PeimEntryPoint
>> +
>> +[Sources]
>> + SgiPlatformPeiLib.c
>> +
>> +[Guids]
>> + gArmSgiPlatformIdDescriptorGuid
>> +
>> +[Ppis]
>> + gHwConfigDtInfoPpiGuid
>> +
>> +[Depex]
>> + TRUE
>> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
>> index d995937..ea9f6c5 100644
>> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
>> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
>> @@ -29,9 +29,14 @@
>> Include # Root include for the package
>>
>> [Guids.common]
>> + # ARM Sgi Platform ID descriptor
>> + gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } }
>> gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
>> gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } }
>>
>> [PcdsFeatureFlag.common]
>> # Set this PCD to TRUE to enable virtio support.
>> gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001
>> +
>> +[Ppis]
>> + gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB
2018-06-15 6:19 ` Ard Biesheuvel
@ 2018-06-15 9:03 ` Leif Lindholm
0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2018-06-15 9:03 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Chandni Cherukuri, edk2-devel@lists.01.org
On Fri, Jun 15, 2018 at 08:19:35AM +0200, Ard Biesheuvel wrote:
> On 15 June 2018 at 08:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> >> new file mode 100644
> >> index 0000000..502d6ac
> >> --- /dev/null
> >> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> >> @@ -0,0 +1,40 @@
> >> +#
> >> +# Copyright (c) 2018, ARM Limited. All rights reserved.
> >> +#
> >> +# This program and the accompanying materials are licensed and made available
> >> +# under the terms and conditions of the BSD License which accompanies this
> >> +# distribution. The full text of the license may be found at
> >> +# http://opensource.org/licenses/bsd-license.php
> >> +#
> >> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> >> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> +#
> >> +
> >> +[Defines]
> >> + INF_VERSION = 0x0001001A
> >> + BASE_NAME = SgiPlatformIdPeiLib
>
> I forgot: this is a module not a library. So it should be called
> SgiPlatformIdPei or SgiPlatformIdPeim not ...Lib
A grep through the edk2 tree suggests that:
- the directory is usually called *Pei
- .inf/.uni usually *Pei.*
- entry point and source files *Peim*
/
Leif
> >> + FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
> >> + MODULE_TYPE = PEIM
> >> + VERSION_STRING = 1.0
> >> + ENTRY_POINT = SgiPlatformIdPeim
> >> +
> >> +[Packages]
> >> + EmbeddedPkg/EmbeddedPkg.dec
> >> + MdePkg/MdePkg.dec
> >> + Platform/ARM/SgiPkg/SgiPlatform.dec
> >> +
> >> +[LibraryClasses]
> >> + FdtLib
> >> + PeimEntryPoint
> >> +
> >> +[Sources]
> >> + SgiPlatformPeiLib.c
> >> +
> >> +[Guids]
> >> + gArmSgiPlatformIdDescriptorGuid
> >> +
> >> +[Ppis]
> >> + gHwConfigDtInfoPpiGuid
> >> +
> >> +[Depex]
> >> + TRUE
> >> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> >> index d995937..ea9f6c5 100644
> >> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> >> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> >> @@ -29,9 +29,14 @@
> >> Include # Root include for the package
> >>
> >> [Guids.common]
> >> + # ARM Sgi Platform ID descriptor
> >> + gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } }
> >> gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
> >> gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } }
> >>
> >> [PcdsFeatureFlag.common]
> >> # Set this PCD to TRUE to enable virtio support.
> >> gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001
> >> +
> >> +[Ppis]
> >> + gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> >> --
> >> 2.7.4
> >>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB
2018-06-15 5:55 ` [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB Chandni Cherukuri
2018-06-15 6:16 ` Ard Biesheuvel
@ 2018-06-15 10:18 ` Leif Lindholm
2018-06-15 12:10 ` chandni cherukuri
1 sibling, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2018-06-15 10:18 UTC (permalink / raw)
To: Chandni Cherukuri; +Cc: edk2-devel, ard.biesheuvel
Just some minor formatting points in addition to the comment on Ard's
reply..
On Fri, Jun 15, 2018 at 11:25:31AM +0530, Chandni Cherukuri wrote:
> On SGI platforms, the trusted firmware executes prior to
> the SEC phase. It supplies to the SEC phase a pointer to
> a fdt, that contains platform specific information such as
> part number and config number of the SGI platform. The
> platform code that executes during the SEC phase creates a
> PPI that allows access to other PEIMs to obtain a copy of the
> fdt pointer.
>
> Further, during the PEI phase, a Platform ID PEIM installs a
> Platform ID HOB. The Platform ID HOB can be used by DXE
> drivers to identify the platform it is executing on.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> ---
> .../SgiPkg/Library/PlatformLib/AArch64/Helper.S | 15 ++-
> .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 ++
> .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +
> .../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c | 108 +++++++++++++++++++++
> .../SgiPlatformPeiLib/SgiPlatformPeiLib.inf | 40 ++++++++
> Platform/ARM/SgiPkg/SgiPlatform.dec | 5 +
> 6 files changed, 177 insertions(+), 4 deletions(-)
> create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> index dab6c77..80b93ea 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> @@ -22,15 +22,22 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> +GCC_ASM_EXPORT(HwConfigDtBlob)
> +
> +HwConfigDtBlob: .long 0
>
> //
> // First platform specific function to be called in the PEI phase
> //
> -// This function is actually the first function called by the PrePi
> -// or PrePeiCore modules. It allows to retrieve arguments passed to
> -// the UEFI firmware through the CPU registers.
> -//
> +// The trusted firmware passed the hw config DT blob in x1 register.
> +// Keep a copy of it in a global variable.
> +//VOID
> +//ArmPlatformPeiBootAction (
> +// VOID
> +// );
> ASM_PFX(ArmPlatformPeiBootAction):
> + ldr x10, =HwConfigDtBlob
> + str x1, [x10]
> ret
>
> //UINTN
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> index ea3201a..54860e0 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> @@ -15,6 +15,10 @@
> #include <Library/ArmPlatformLib.h>
> #include <Library/BaseLib.h>
> #include <Ppi/ArmMpCoreInfo.h>
> +#include <SgiPlatform.h>
> +
> +extern UINT64 HwConfigDtBlob;
> +STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
>
> STATIC ARM_CORE_INFO mCoreInfoTable[] = {
> {
> @@ -36,6 +40,7 @@ ArmPlatformInitialize (
> IN UINTN MpId
> )
> {
> + mHwConfigDtInfoPpi.HwConfigDtAddr = &HwConfigDtBlob;
> return RETURN_SUCCESS;
> }
>
> @@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
> EFI_PEI_PPI_DESCRIPTOR_PPI,
> &gArmMpCoreInfoPpiGuid,
> &mMpCoreInfoPpi
> + },
> + {
> + EFI_PEI_PPI_DESCRIPTOR_PPI,
> + &gHwConfigDtInfoPpiGuid,
> + &mHwConfigDtInfoPpi
> }
> };
>
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> index 42e14d5..5d4bf72 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> @@ -61,7 +61,10 @@
> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
> [Guids]
> + gArmSgiPlatformIdDescriptorGuid
> gEfiHobListGuid ## CONSUMES ## SystemTable
> + gFdtTableGuid
>
> [Ppis]
> gArmMpCoreInfoPpiGuid
> + gHwConfigDtInfoPpiGuid
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> new file mode 100644
> index 0000000..f6446e6
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> @@ -0,0 +1,108 @@
> +/** @file
> +*
> +* Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +* This program and the accompanying materials are licensed and made available
> +* under the terms and conditions of the BSD License which accompanies this
> +* distribution. The full text of the license may be found at
> +* http://opensource.org/licenses/bsd-license.php
> +*
> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PeimEntryPoint.h>
> +#include <Library/PeiServicesLib.h>
> +#include <libfdt.h>
> +#include <SgiPlatform.h>
> +
> +/**
> +
> + This function returns the Platform ID of the platform
> +
> + This functions locates the HwConfig PPI and gets the base address of HW CONFIG
> + DT from which the platform ID is obtained using FDT helper functions
> +
> + @return returns the platform ID on success else returns 0 on error
> +
> +**/
> +
> +STATIC
> +UINT32
> +GetSgiPlatformId (
> + VOID
> +)
> +{
> + CONST UINT32 *Property;
> + INT32 Offset;
> + CONST VOID *HwCfgDtBlob;
> + SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi;
> + EFI_STATUS Status;
> +
> + Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, (VOID**)&HwConfigInfoPpi);
93 characters. Please run BaseTools/Scripts/PatchCheck.py.
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "PeiServicesLocatePpi failed with error %r\n", Status));
> + return 0;
> + }
> +
> + HwCfgDtBlob = (VOID *)(*(HwConfigInfoPpi->HwConfigDtAddr));
> + if (fdt_check_header (HwCfgDtBlob) != 0 ) {
Trailing space after 0.
> + DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed as HW_CONFIG\n", HwCfgDtBlob));
> + return 0;
> + }
> +
> + Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id");
> + if (Offset == -FDT_ERR_NOTFOUND) {
> + DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n"));
> + return 0;
> + }
> +
> + Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
> + if (Property == NULL) {
> + DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
> + return 0;
> + }
> +
> + return fdt32_to_cpu (*Property);
> +}
> +
> +/**
> +
> + This function creates a Platform ID HOB and assigns PlatformId as the
> + HobData
> +
> + @return asserts on error and returns EFI_INVALID_PARAMETER. On success
> + returns EFI_SUCCESS
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SgiPlatformIdPeim (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> +)
> +{
> + SGI_PLATFORM_DESCRIPTOR *HobData;
> +
> + //Create platform descriptor HOB
> + HobData = (SGI_PLATFORM_DESCRIPTOR *)BuildGuidHob (&gArmSgiPlatformIdDescriptorGuid,
> + sizeof (SGI_PLATFORM_DESCRIPTOR));
> +
> + //Get the platform id from the platform specific hw_config device tree
> + if (HobData != NULL) {
> + HobData->PlatformId = GetSgiPlatformId ();
> + if (HobData->PlatformId == 0) {
> + ASSERT (FALSE);
> + return EFI_INVALID_PARAMETER;
> + }
> + } else {
> + DEBUG ((DEBUG_ERROR, "Platform HoB is NULL\n"));
HOB
/
Leif
> + ASSERT (FALSE);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> new file mode 100644
> index 0000000..502d6ac
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> @@ -0,0 +1,40 @@
> +#
> +# Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
> + BASE_NAME = SgiPlatformIdPeiLib
> + FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
> + MODULE_TYPE = PEIM
> + VERSION_STRING = 1.0
> + ENTRY_POINT = SgiPlatformIdPeim
> +
> +[Packages]
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdePkg/MdePkg.dec
> + Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[LibraryClasses]
> + FdtLib
> + PeimEntryPoint
> +
> +[Sources]
> + SgiPlatformPeiLib.c
> +
> +[Guids]
> + gArmSgiPlatformIdDescriptorGuid
> +
> +[Ppis]
> + gHwConfigDtInfoPpiGuid
> +
> +[Depex]
> + TRUE
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> index d995937..ea9f6c5 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -29,9 +29,14 @@
> Include # Root include for the package
>
> [Guids.common]
> + # ARM Sgi Platform ID descriptor
> + gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } }
> gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
> gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } }
>
> [PcdsFeatureFlag.common]
> # Set this PCD to TRUE to enable virtio support.
> gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001
> +
> +[Ppis]
> + gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID
2018-06-15 5:55 ` [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID Chandni Cherukuri
2018-06-15 6:18 ` Ard Biesheuvel
@ 2018-06-15 10:22 ` Leif Lindholm
1 sibling, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2018-06-15 10:22 UTC (permalink / raw)
To: Chandni Cherukuri; +Cc: edk2-devel, ard.biesheuvel
On Fri, Jun 15, 2018 at 11:25:32AM +0530, Chandni Cherukuri wrote:
> Use the platform ID and config ID values from the platform ID
> HOB to choose the right set of ACPI tables to be installed.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> ---
> .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c | 33 ++++++++++++++++++----
> .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 2 ++
> Platform/ARM/SgiPkg/Include/SgiPlatform.h | 24 ++++++++++++++--
> Platform/ARM/SgiPkg/SgiPlatform.dsc | 1 +
> Platform/ARM/SgiPkg/SgiPlatform.fdf | 1 +
> 5 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index edaae5b..d0d5808 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -14,6 +14,8 @@
>
> #include <Library/AcpiLib.h>
> #include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <SgiPlatform.h>
>
> EFI_STATUS
> InitVirtioBlockIo (
> @@ -28,20 +30,41 @@ ArmSgiPkgEntryPoint (
> )
> {
> EFI_STATUS Status;
> + VOID *PlatformIdHob;
> + SGI_PLATFORM_DESCRIPTOR *HobData;
> + UINT32 ConfigId;
> + UINT32 PartNum;
>
> - Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesiFileGuid);
> - if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
> - return Status;
> + PlatformIdHob = GetFirstGuidHob (&gArmSgiPlatformIdDescriptorGuid);
> + if (PlatformIdHob == NULL) {
> + DEBUG ((DEBUG_ERROR, "Platform ID Hob is NULL\n"));
HOB
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + HobData = (SGI_PLATFORM_DESCRIPTOR *)(GET_GUID_HOB_DATA (PlatformIdHob));
> +
> + PartNum = (HobData->PlatformId & SGI_PART_NUM_MASK);
> + ConfigId = ((HobData->PlatformId >> SGI_CONFIG_SHIFT) & SGI_CONFIG_MASK);
> +
> + if ((PartNum == SGI575_PART_NUM) && (ConfigId == SGI575_CONF_NUM)) {
> + Status = LocateAndInstallAcpiFromFv (&gSgi575AcpiTablesiFileGuid);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
> + return Status;
> + }
> + } else {
> + DEBUG ((DEBUG_ERROR, "PlatformDxe: Unsupported Platform Id\n"));
> + return EFI_UNSUPPORTED;
> }
>
> - Status = EFI_REQUEST_UNLOAD_IMAGE;
> if (FeaturePcdGet (PcdVirtioSupported)) {
> Status = InitVirtioBlockIo (ImageHandle);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a: Failed to install Virtio Block device\n",
> __FUNCTION__));
> }
> + } else {
> + Status = EFI_REQUEST_UNLOAD_IMAGE;
> }
>
> return Status;
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index 51ad22f..b6b8209 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -30,10 +30,12 @@
>
> [LibraryClasses]
> AcpiLib
> + HobLib
> UefiDriverEntryPoint
> VirtioMmioDeviceLib
>
> [Guids]
> + gArmSgiPlatformIdDescriptorGuid
> gSgi575AcpiTablesiFileGuid
>
> [FeaturePcd]
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> index 00ca7e9..756954c 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -12,8 +12,8 @@
> *
> **/
>
> -#ifndef __SGI_PLATFORM_H__
> -#define __SGI_PLATFORM_H__
> +#ifndef SGI_PLATFORM_H_
> +#define SGI_PLATFORM_H_
Non-functional change. Please submit as separate change or not at all.
/
Leif
>
> /***********************************************************************************
> // Platform Memory Map
> @@ -68,4 +68,22 @@
> #define SGI_SYSPH_SYS_REG_FLASH 0x4C
> #define SGI_SYSPH_SYS_REG_FLASH_RWEN 0x1
>
> -#endif // __SGI_PLATFORM_H__
> +// SGI575_VERSION values
> +#define SGI575_CONF_NUM 0x3
> +#define SGI575_PART_NUM 0x783
> +
> +#define SGI_CONFIG_MASK 0x0F
> +#define SGI_CONFIG_SHIFT 0x1C
> +#define SGI_PART_NUM_MASK 0xFFF
> +
> +//HwConfig DT structure^M
> +typedef struct {
> + UINT64 *HwConfigDtAddr;
> +} SGI_HW_CONFIG_INFO_PPI;
> +
> +// ARM platform description data.
> +typedef struct {
> + UINTN PlatformId;
> +} SGI_PLATFORM_DESCRIPTOR;
> +
> +#endif // SGI_PLATFORM_H_
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> index a56175e..f571897 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -199,6 +199,7 @@
> <LibraryClasses>
> NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> }
> + Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>
> #
> # DXE
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> index 17cdf48..05203a8 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> @@ -220,6 +220,7 @@ READ_LOCK_STATUS = TRUE
> INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> + INF Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
>
> FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB
2018-06-15 10:18 ` Leif Lindholm
@ 2018-06-15 12:10 ` chandni cherukuri
0 siblings, 0 replies; 10+ messages in thread
From: chandni cherukuri @ 2018-06-15 12:10 UTC (permalink / raw)
To: Leif Lindholm; +Cc: Chandni Cherukuri, edk2-devel, ard.biesheuvel
On Fri, Jun 15, 2018 at 3:48 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> Just some minor formatting points in addition to the comment on Ard's
> reply..
>
> On Fri, Jun 15, 2018 at 11:25:31AM +0530, Chandni Cherukuri wrote:
> > On SGI platforms, the trusted firmware executes prior to
> > the SEC phase. It supplies to the SEC phase a pointer to
> > a fdt, that contains platform specific information such as
> > part number and config number of the SGI platform. The
> > platform code that executes during the SEC phase creates a
> > PPI that allows access to other PEIMs to obtain a copy of the
> > fdt pointer.
> >
> > Further, during the PEI phase, a Platform ID PEIM installs a
> > Platform ID HOB. The Platform ID HOB can be used by DXE
> > drivers to identify the platform it is executing on.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> > ---
> > .../SgiPkg/Library/PlatformLib/AArch64/Helper.S | 15 ++-
> > .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 10 ++
> > .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +
> > .../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c | 108 +++++++++++++++++++++
> > .../SgiPlatformPeiLib/SgiPlatformPeiLib.inf | 40 ++++++++
> > Platform/ARM/SgiPkg/SgiPlatform.dec | 5 +
> > 6 files changed, 177 insertions(+), 4 deletions(-)
> > create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> > create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> >
> > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> > index dab6c77..80b93ea 100644
> > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> > @@ -22,15 +22,22 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> > GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> > GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> > GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> > +GCC_ASM_EXPORT(HwConfigDtBlob)
> > +
> > +HwConfigDtBlob: .long 0
> >
> > //
> > // First platform specific function to be called in the PEI phase
> > //
> > -// This function is actually the first function called by the PrePi
> > -// or PrePeiCore modules. It allows to retrieve arguments passed to
> > -// the UEFI firmware through the CPU registers.
> > -//
> > +// The trusted firmware passed the hw config DT blob in x1 register.
> > +// Keep a copy of it in a global variable.
> > +//VOID
> > +//ArmPlatformPeiBootAction (
> > +// VOID
> > +// );
> > ASM_PFX(ArmPlatformPeiBootAction):
> > + ldr x10, =HwConfigDtBlob
> > + str x1, [x10]
> > ret
> >
> > //UINTN
> > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> > index ea3201a..54860e0 100644
> > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
> > @@ -15,6 +15,10 @@
> > #include <Library/ArmPlatformLib.h>
> > #include <Library/BaseLib.h>
> > #include <Ppi/ArmMpCoreInfo.h>
> > +#include <SgiPlatform.h>
> > +
> > +extern UINT64 HwConfigDtBlob;
> > +STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
> >
> > STATIC ARM_CORE_INFO mCoreInfoTable[] = {
> > {
> > @@ -36,6 +40,7 @@ ArmPlatformInitialize (
> > IN UINTN MpId
> > )
> > {
> > + mHwConfigDtInfoPpi.HwConfigDtAddr = &HwConfigDtBlob;
> > return RETURN_SUCCESS;
> > }
> >
> > @@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
> > EFI_PEI_PPI_DESCRIPTOR_PPI,
> > &gArmMpCoreInfoPpiGuid,
> > &mMpCoreInfoPpi
> > + },
> > + {
> > + EFI_PEI_PPI_DESCRIPTOR_PPI,
> > + &gHwConfigDtInfoPpiGuid,
> > + &mHwConfigDtInfoPpi
> > }
> > };
> >
> > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> > index 42e14d5..5d4bf72 100644
> > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> > @@ -61,7 +61,10 @@
> > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> >
> > [Guids]
> > + gArmSgiPlatformIdDescriptorGuid
> > gEfiHobListGuid ## CONSUMES ## SystemTable
> > + gFdtTableGuid
> >
> > [Ppis]
> > gArmMpCoreInfoPpiGuid
> > + gHwConfigDtInfoPpiGuid
> > diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> > new file mode 100644
> > index 0000000..f6446e6
> > --- /dev/null
> > +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
> > @@ -0,0 +1,108 @@
> > +/** @file
> > +*
> > +* Copyright (c) 2018, ARM Limited. All rights reserved.
> > +*
> > +* This program and the accompanying materials are licensed and made available
> > +* under the terms and conditions of the BSD License which accompanies this
> > +* distribution. The full text of the license may be found at
> > +* http://opensource.org/licenses/bsd-license.php
> > +*
> > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#include <Library/DebugLib.h>
> > +#include <Library/HobLib.h>
> > +#include <Library/PeimEntryPoint.h>
> > +#include <Library/PeiServicesLib.h>
> > +#include <libfdt.h>
> > +#include <SgiPlatform.h>
> > +
> > +/**
> > +
> > + This function returns the Platform ID of the platform
> > +
> > + This functions locates the HwConfig PPI and gets the base address of HW CONFIG
> > + DT from which the platform ID is obtained using FDT helper functions
> > +
> > + @return returns the platform ID on success else returns 0 on error
> > +
> > +**/
> > +
> > +STATIC
> > +UINT32
> > +GetSgiPlatformId (
> > + VOID
> > +)
> > +{
> > + CONST UINT32 *Property;
> > + INT32 Offset;
> > + CONST VOID *HwCfgDtBlob;
> > + SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi;
> > + EFI_STATUS Status;
> > +
> > + Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, (VOID**)&HwConfigInfoPpi);
>
> 93 characters. Please run BaseTools/Scripts/PatchCheck.py.
>
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "PeiServicesLocatePpi failed with error %r\n", Status));
> > + return 0;
> > + }
> > +
> > + HwCfgDtBlob = (VOID *)(*(HwConfigInfoPpi->HwConfigDtAddr));
> > + if (fdt_check_header (HwCfgDtBlob) != 0 ) {
>
> Trailing space after 0.
>
> > + DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed as HW_CONFIG\n", HwCfgDtBlob));
> > + return 0;
> > + }
> > +
> > + Offset = fdt_subnode_offset (HwCfgDtBlob, 0, "system-id");
> > + if (Offset == -FDT_ERR_NOTFOUND) {
> > + DEBUG ((DEBUG_ERROR, "Invalid DTB : system-id node not found\n"));
> > + return 0;
> > + }
> > +
> > + Property = fdt_getprop (HwCfgDtBlob, Offset, "platform-id", NULL);
> > + if (Property == NULL) {
> > + DEBUG ((DEBUG_ERROR, "Platform Id is NULL\n"));
> > + return 0;
> > + }
> > +
> > + return fdt32_to_cpu (*Property);
> > +}
> > +
> > +/**
> > +
> > + This function creates a Platform ID HOB and assigns PlatformId as the
> > + HobData
> > +
> > + @return asserts on error and returns EFI_INVALID_PARAMETER. On success
> > + returns EFI_SUCCESS
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SgiPlatformIdPeim (
> > + IN EFI_PEI_FILE_HANDLE FileHandle,
> > + IN CONST EFI_PEI_SERVICES **PeiServices
> > +)
> > +{
> > + SGI_PLATFORM_DESCRIPTOR *HobData;
> > +
> > + //Create platform descriptor HOB
> > + HobData = (SGI_PLATFORM_DESCRIPTOR *)BuildGuidHob (&gArmSgiPlatformIdDescriptorGuid,
> > + sizeof (SGI_PLATFORM_DESCRIPTOR));
> > +
> > + //Get the platform id from the platform specific hw_config device tree
> > + if (HobData != NULL) {
> > + HobData->PlatformId = GetSgiPlatformId ();
> > + if (HobData->PlatformId == 0) {
> > + ASSERT (FALSE);
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + } else {
> > + DEBUG ((DEBUG_ERROR, "Platform HoB is NULL\n"));
>
> HOB
>
> /
> Leif
Hi Ard, Leif,
Thank you for your comments. All the changes as per your comments have
been made and updated patches have been posted for your review.
Thanks,
Chandni.
>
> > + ASSERT (FALSE);
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> > new file mode 100644
> > index 0000000..502d6ac
> > --- /dev/null
> > +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
> > @@ -0,0 +1,40 @@
> > +#
> > +# Copyright (c) 2018, ARM Limited. All rights reserved.
> > +#
> > +# This program and the accompanying materials are licensed and made available
> > +# under the terms and conditions of the BSD License which accompanies this
> > +# distribution. The full text of the license may be found at
> > +# http://opensource.org/licenses/bsd-license.php
> > +#
> > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +#
> > +
> > +[Defines]
> > + INF_VERSION = 0x0001001A
> > + BASE_NAME = SgiPlatformIdPeiLib
> > + FILE_GUID = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
> > + MODULE_TYPE = PEIM
> > + VERSION_STRING = 1.0
> > + ENTRY_POINT = SgiPlatformIdPeim
> > +
> > +[Packages]
> > + EmbeddedPkg/EmbeddedPkg.dec
> > + MdePkg/MdePkg.dec
> > + Platform/ARM/SgiPkg/SgiPlatform.dec
> > +
> > +[LibraryClasses]
> > + FdtLib
> > + PeimEntryPoint
> > +
> > +[Sources]
> > + SgiPlatformPeiLib.c
> > +
> > +[Guids]
> > + gArmSgiPlatformIdDescriptorGuid
> > +
> > +[Ppis]
> > + gHwConfigDtInfoPpiGuid
> > +
> > +[Depex]
> > + TRUE
> > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> > index d995937..ea9f6c5 100644
> > --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> > @@ -29,9 +29,14 @@
> > Include # Root include for the package
> >
> > [Guids.common]
> > + # ARM Sgi Platform ID descriptor
> > + gArmSgiPlatformIdDescriptorGuid = { 0xf56f152a, 0xad2a, 0x11e6, { 0xb1, 0xa7, 0x00, 0x50, 0x56, 0x3c, 0x44, 0xcc } }
> > gArmSgiTokenSpaceGuid = { 0x577d6941, 0xaea1, 0x40b4, { 0x90, 0x93, 0x2a, 0x86, 0x61, 0x72, 0x5a, 0x57 } }
> > gSgi575AcpiTablesiFileGuid = { 0xc712719a, 0x0aaf, 0x438c, { 0x9c, 0xdd, 0x35, 0xab, 0x4d, 0x60, 0x20, 0x7d } }
> >
> > [PcdsFeatureFlag.common]
> > # Set this PCD to TRUE to enable virtio support.
> > gArmSgiTokenSpaceGuid.PcdVirtioSupported|TRUE|BOOLEAN|0x00000001
> > +
> > +[Ppis]
> > + gHwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> > --
> > 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] 10+ messages in thread
end of thread, other threads:[~2018-06-15 12:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-15 5:55 [PATCH 0/2] ARM Dynamic Configuration Chandni Cherukuri
2018-06-15 5:55 ` [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB Chandni Cherukuri
2018-06-15 6:16 ` Ard Biesheuvel
2018-06-15 6:19 ` Ard Biesheuvel
2018-06-15 9:03 ` Leif Lindholm
2018-06-15 10:18 ` Leif Lindholm
2018-06-15 12:10 ` chandni cherukuri
2018-06-15 5:55 ` [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID Chandni Cherukuri
2018-06-15 6:18 ` Ard Biesheuvel
2018-06-15 10:22 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox