[PATCH edk2-platforms v2 0/2] ARM Dynamic Configuration

[PATCH edk2-platforms v2 0/2] ARM Dynamic Configuration

[Chandni Cherukuri]

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

 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c         |  33 +++++-
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf       |   2 +
 Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h               |  23 ++++
 Platform/ARM/SgiPkg/Include/SgiPlatform.h                     |  13 +++
 Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  13 ++-
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c         |  10 ++
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf       |   3 +
 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf |  40 +++++++
 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c  | 112 ++++++++++++++++++++
 Platform/ARM/SgiPkg/SgiPlatform.dec                           |   5 +
 Platform/ARM/SgiPkg/SgiPlatform.dsc                           |   1 +
 Platform/ARM/SgiPkg/SgiPlatform.fdf                           |   1 +
 12 files changed, 247 insertions(+), 9 deletions(-)
 create mode 100644 Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
 create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
 create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c

-- 
2.7.4

[PATCH edk2-platforms v2 1/2] Platform/ARM/Sgi: Install a Platform ID HOB

[Chandni Cherukuri]

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>
---
 Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h               |  23 ++++
 Platform/ARM/SgiPkg/Include/SgiPlatform.h                     |  13 +++
 Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  13 ++-
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c         |  10 ++
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf       |   3 +
 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf |  40 +++++++
 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c  | 112 ++++++++++++++++++++
 Platform/ARM/SgiPkg/SgiPlatform.dec                           |   5 +
 8 files changed, 215 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
new file mode 100644
index 0000000..5ffc69d
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
@@ -0,0 +1,23 @@
+/** @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.
+*
+**/
+
+#ifndef  SGI_PLATFORMID_PPI_
+#define  SGI_PLATFORMID_PPI_
+
+//HwConfig DT structure
+typedef struct {
+  UINT64                  *HwConfigDtAddr;
+} SGI_HW_CONFIG_INFO_PPI;
+
+#endif
diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index 00ca7e9..1454018 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -68,4 +68,17 @@
 #define SGI_SYSPH_SYS_REG_FLASH                   0x4C
 #define SGI_SYSPH_SYS_REG_FLASH_RWEN              0x1
 
+// 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
+
+// ARM platform description data.
+typedef struct {
+  UINTN  PlatformId;
+} SGI_PLATFORM_DESCRIPTOR;
+
 #endif // __SGI_PLATFORM_H__
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
index dab6c77..3662266 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
@@ -22,15 +22,20 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
 GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
 GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
 GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
+GCC_ASM_IMPORT(HwConfigDtBlob)
 
 //
 // 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):
+  adr  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..c9ddbfb 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 <Ppi/SgiPlatformId.h>
+
+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/SgiPlatformPei/SgiPlatformPei.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
new file mode 100644
index 0000000..3d1bab7
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.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                      = SgiPlatformIdPei
+  FILE_GUID                      = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = SgiPlatformPeim
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+  FdtLib
+  PeimEntryPoint
+
+[Sources]
+  SgiPlatformPeim.c
+
+[Guids]
+  gArmSgiPlatformIdDescriptorGuid
+
+[Ppis]
+  gHwConfigDtInfoPpiGuid
+
+[Depex]
+  TRUE
diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
new file mode 100644
index 0000000..77499a6
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
@@ -0,0 +1,112 @@
+/** @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 <Ppi/SgiPlatformId.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\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
+SgiPlatformPeim (
+ 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) {
+    DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
+    ASSERT (FALSE);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  HobData->PlatformId = GetSgiPlatformId ();
+  if (HobData->PlatformId == 0) {
+    ASSERT (FALSE);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  return EFI_SUCCESS;
+}
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

[PATCH edk2-platforms v2 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID

[Chandni Cherukuri]

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>
---
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 33 +++++++++++++++++---
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  2 ++
 Platform/ARM/SgiPkg/SgiPlatform.dsc                     |  1 +
 Platform/ARM/SgiPkg/SgiPlatform.fdf                     |  1 +
 4 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
index edaae5b..f077a2c 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/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
index a56175e..7b8e051 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/SgiPlatformPei/SgiPlatformPei.inf
 
   #
   # DXE
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
index 17cdf48..0e5739e 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/SgiPlatformPei/SgiPlatformPei.inf
 
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
     SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
-- 
2.7.4

Re: [PATCH edk2-platforms v2 0/2] ARM Dynamic Configuration

[Leif Lindholm]

On Fri, Jun 15, 2018 at 05:30:00PM +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 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.

It's helpful for subsequent revisions to keep a revision history in
cover letter.

Changes since v1:
- ...
- ...
- ...

Kept around and added to for subsequent revisions.

However, you've addressed all of my comments on v1, so for my part -
for the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

But we want to hear back from Ard as well.

/
    Leif

> Chandni Cherukuri (2):
>   Platform/ARM/Sgi: Install a Platform ID HOB
>   Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID
> 
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c         |  33 +++++-
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf       |   2 +
>  Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h               |  23 ++++
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h                     |  13 +++
>  Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  13 ++-
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c         |  10 ++
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf       |   3 +
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf |  40 +++++++
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c  | 112 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/SgiPlatform.dec                           |   5 +
>  Platform/ARM/SgiPkg/SgiPlatform.dsc                           |   1 +
>  Platform/ARM/SgiPkg/SgiPlatform.fdf                           |   1 +
>  12 files changed, 247 insertions(+), 9 deletions(-)
>  create mode 100644 Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
>  create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
>  create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> 
> -- 
> 2.7.4
> 
> 

Re: [PATCH edk2-platforms v2 1/2] Platform/ARM/Sgi: Install a Platform ID HOB

[Ard Biesheuvel]

On 15 June 2018 at 14:00, 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>
> ---
>  Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h               |  23 ++++
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h                     |  13 +++
>  Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  13 ++-
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c         |  10 ++
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf       |   3 +
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf |  40 +++++++
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c  | 112 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/SgiPlatform.dec                           |   5 +
>  8 files changed, 215 insertions(+), 4 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
> new file mode 100644
> index 0000000..5ffc69d
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
> @@ -0,0 +1,23 @@
> +/** @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.
> +*
> +**/
> +
> +#ifndef  SGI_PLATFORMID_PPI_
> +#define  SGI_PLATFORMID_PPI_
> +
> +//HwConfig DT structure
> +typedef struct {
> +  UINT64                  *HwConfigDtAddr;
> +} SGI_HW_CONFIG_INFO_PPI;
> +
> +#endif
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> index 00ca7e9..1454018 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -68,4 +68,17 @@
>  #define SGI_SYSPH_SYS_REG_FLASH                   0x4C
>  #define SGI_SYSPH_SYS_REG_FLASH_RWEN              0x1
>
> +// 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
> +
> +// ARM platform description data.
> +typedef struct {
> +  UINTN  PlatformId;
> +} SGI_PLATFORM_DESCRIPTOR;
> +
>  #endif // __SGI_PLATFORM_H__
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> index dab6c77..3662266 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
> @@ -22,15 +22,20 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
>  GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
>  GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
>  GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> +GCC_ASM_IMPORT(HwConfigDtBlob)
>
>  //
>  // 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):
> +  adr  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..c9ddbfb 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 <Ppi/SgiPlatformId.h>
> +
> +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/SgiPlatformPei/SgiPlatformPei.inf b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
> new file mode 100644
> index 0000000..3d1bab7
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.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                      = SgiPlatformIdPei
> +  FILE_GUID                      = a9936f3e-6a1b-11e8-8ce0-fffe69b86863
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = SgiPlatformPeim
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[LibraryClasses]
> +  FdtLib
> +  PeimEntryPoint
> +
> +[Sources]
> +  SgiPlatformPeim.c
> +
> +[Guids]
> +  gArmSgiPlatformIdDescriptorGuid
> +
> +[Ppis]
> +  gHwConfigDtInfoPpiGuid
> +
> +[Depex]
> +  TRUE

This driver should depex on gHwConfigDtInfoPpiGuid, given that it will
complain if it does not find it.

It seems a bit convoluted to install a PPI first, and then invoke it
to populate the HOB. OTOH, other PEIMs may need the platform ID as
well, so I'm fine with keeping it.

> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> new file mode 100644
> index 0000000..77499a6
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> @@ -0,0 +1,112 @@
> +/** @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 <Ppi/SgiPlatformId.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));

This need an intermediate (UINTN) cast, or instead, you can use a
UINTN* for the address.
In fact, why are you using a pointer here rather than the address of
the DT itself?

> +  if (fdt_check_header (HwCfgDtBlob) != 0) {
> +    DEBUG ((DEBUG_ERROR, "Invalid DTB file %p passed\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
> +SgiPlatformPeim (
> + 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) {
> +    DEBUG ((DEBUG_ERROR, "Platform HOB is NULL\n"));
> +    ASSERT (FALSE);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  HobData->PlatformId = GetSgiPlatformId ();
> +  if (HobData->PlatformId == 0) {
> +    ASSERT (FALSE);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> 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
>

Re: [PATCH edk2-platforms v2 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID

[Ard Biesheuvel]

On 15 June 2018 at 14:00, 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>
> ---
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 33 +++++++++++++++++---
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  2 ++
>  Platform/ARM/SgiPkg/SgiPlatform.dsc                     |  1 +
>  Platform/ARM/SgiPkg/SgiPlatform.fdf                     |  1 +
>  4 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index edaae5b..f077a2c 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;
>    }
>

This is an unrelated (and unnecessary) change.


>    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/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> index a56175e..7b8e051 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/SgiPlatformPei/SgiPlatformPei.inf
>
>    #
>    # DXE
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> index 17cdf48..0e5739e 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/SgiPlatformPei/SgiPlatformPei.inf
>
>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> --
> 2.7.4
>

Re: [PATCH edk2-platforms v2 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID

[chandni cherukuri]

On Fri, Jun 15, 2018 at 6:35 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 15 June 2018 at 14:00, 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>
>> ---
>>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 33 +++++++++++++++++---
>>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  2 ++
>>  Platform/ARM/SgiPkg/SgiPlatform.dsc                     |  1 +
>>  Platform/ARM/SgiPkg/SgiPlatform.fdf                     |  1 +
>>  4 files changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
>> index edaae5b..f077a2c 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;
>>    }
>>
>
> This is an unrelated (and unnecessary) change.

removed this unrelated change in v3 patchset
>
>
>>    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/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
>> index a56175e..7b8e051 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/SgiPlatformPei/SgiPlatformPei.inf
>>
>>    #
>>    # DXE
>> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
>> index 17cdf48..0e5739e 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/SgiPlatformPei/SgiPlatformPei.inf
>>
>>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>> --
>> 2.7.4
>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH edk2-platforms v2 0/2] ARM Dynamic Configuration

[chandni cherukuri]

On Fri, Jun 15, 2018 at 6:10 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Jun 15, 2018 at 05:30:00PM +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 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.
>
> It's helpful for subsequent revisions to keep a revision history in
> cover letter.
>
> Changes since v1:
> - ...
> - ...
> - ...
>
> Kept around and added to for subsequent revisions.
>
> However, you've addressed all of my comments on v1, so for my part -
> for the series:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> But we want to hear back from Ard as well.
>
> /
>     Leif

Thanks Leif for reviewing the patches. Have addressed the comments from Ard
and submitted v3 patchset and included "Changes since v1:" in the cover letter.

>
>> Chandni Cherukuri (2):
>>   Platform/ARM/Sgi: Install a Platform ID HOB
>>   Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID
>>
>>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c         |  33 +++++-
>>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf       |   2 +
>>  Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h               |  23 ++++
>>  Platform/ARM/SgiPkg/Include/SgiPlatform.h                     |  13 +++
>>  Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S      |  13 ++-
>>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c         |  10 ++
>>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf       |   3 +
>>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf |  40 +++++++
>>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c  | 112 ++++++++++++++++++++
>>  Platform/ARM/SgiPkg/SgiPlatform.dec                           |   5 +
>>  Platform/ARM/SgiPkg/SgiPlatform.dsc                           |   1 +
>>  Platform/ARM/SgiPkg/SgiPlatform.fdf                           |   1 +
>>  12 files changed, 247 insertions(+), 9 deletions(-)
>>  create mode 100644 Platform/ARM/SgiPkg/Include/Ppi/SgiPlatformId.h
>>  create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPei.inf
>>  create mode 100644 Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
>>
>> --
>> 2.7.4
>>
>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel