Re: [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB

[chandni cherukuri <chandni.cherukuri@arm.com>]

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