Re: [PATCH 08/10] StandaloneMmPkg: Add an AArch64 specific entry point library.

[Achin Gupta <achin.gupta@arm.com>]

Hi Sughosh,

Thanks a lot for picking this up. CIL..

On Tue, Jul 03, 2018 at 03:12:39PM +0530, Supreeth Venkatesh wrote:
> The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard
> Platforms and is initialised during the SEC phase. ARM Trusted firmware
> in EL3 is responsible for initialising the architectural context for
> S-EL0 and loading the Standalone MM image. The memory allocated to this
> image is marked as RO+X. Heap memory is marked as RW+XN.
> 
> Certain actions have to be completed prior to executing the generic code
> in the Standalone MM Core module. These are:
> 
> 1. Memory permission attributes for each section of the Standalone MM
>    Core module need to be changed prior to accessing any RW data.
> 
> 2. A Hob list has to be created with information that allows the MM
>    environment to initialise and dispatch drivers.
> 
> Furthermore, this module is responsible for handing over runtime MM
> events to the Standalone MM CPU driver and returning control to ARM
> Trusted Firmware upon event completion. Hence it needs to know the CPU
> driver entry point.
> 
> This patch implements an entry point module that ARM Trusted Firmware
> jumps to in S-EL0. It then performs the above actions before calling the
> Standalone MM Foundation entry point and handling subsequent MM events.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Jiewen Yao <Jiewen.yao@intel.com>
> Cc: Achin Gupta <achin.gupta@arm.com>
> ---
>  .../Library/AArch64/StandaloneMmCoreEntryPoint.h   | 214 +++++++++++++++
>  .../AArch64/CreateHobList.c                        | 200 ++++++++++++++
>  .../AArch64/SetPermissions.c                       | 275 ++++++++++++++++++++
>  .../AArch64/StandaloneMmCoreEntryPoint.c           | 287 +++++++++++++++++++++
>  .../StandaloneMmCoreEntryPoint.inf                 |  55 ++++
>  5 files changed, 1031 insertions(+)
>  create mode 100644 StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
>  create mode 100644 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
>  create mode 100644 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
>  create mode 100644 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
>  create mode 100644 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> 
> diff --git a/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
> new file mode 100644
> index 0000000..6eb74af
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
> @@ -0,0 +1,214 @@
> +/** @file
> +  Entry point to the Standalone MM Foundation when initialized during the SEC
> +  phase on ARM platforms
> +
> +Copyright (c) 2017 - 2018, ARM Ltd. All rights reserved.<BR>
> +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 __STANDALONEMMCORE_ENTRY_POINT_H__
> +#define __STANDALONEMMCORE_ENTRY_POINT_H__
> +
> +#include <Library/PeCoffLib.h>
> +#include <Library/FvLib.h>
> +
> +#define CPU_INFO_FLAG_PRIMARY_CPU  0x00000001
> +
> +typedef struct {
> +  UINT8  Type;       /* type of the structure */
> +  UINT8  Version;    /* version of this structure */
> +  UINT16 Size;      /* size of this structure in bytes */
> +  UINT32 Attr;      /* attributes: unused bits SBZ */
> +} EFI_PARAM_HEADER;
> +
> +typedef struct {
> +  UINT64 Mpidr;
> +  UINT32 LinearId;
> +  UINT32 Flags;
> +} EFI_SECURE_PARTITION_CPU_INFO;
> +
> +typedef struct {
> +  EFI_PARAM_HEADER              Header;
> +  UINT64                        SpMemBase;
> +  UINT64                        SpMemLimit;
> +  UINT64                        SpImageBase;
> +  UINT64                        SpStackBase;
> +  UINT64                        SpHeapBase;
> +  UINT64                        SpNsCommBufBase;
> +  UINT64                        SpSharedBufBase;
> +  UINT64                        SpImageSize;
> +  UINT64                        SpPcpuStackSize;
> +  UINT64                        SpHeapSize;
> +  UINT64                        SpNsCommBufSize;
> +  UINT64                        SpPcpuSharedBufSize;
> +  UINT32                        NumSpMemRegions;
> +  UINT32                        NumCpus;
> +  EFI_SECURE_PARTITION_CPU_INFO *CpuInfo;
> +} EFI_SECURE_PARTITION_BOOT_INFO;
> +
> +typedef
> +EFI_STATUS
> +(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
> +  IN UINTN EventId,
> +  IN UINTN CpuNumber,
> +  IN UINTN NsCommBufferAddr
> +  );
> +
> +typedef struct {
> +  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT *ArmTfCpuDriverEpPtr;
> +} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
> +
> +typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
> +  IN  EFI_PHYSICAL_ADDRESS      BaseAddress,
> +  IN  UINT64                    Length
> +  );
> +
> +/**
> +  Privileged firmware assigns RO & Executable attributes to all memory occupied
> +  by the Boot Firmware Volume. This function sets the correct permissions of
> +  sections in the Standalone MM Core module to be able to access RO and RW data
> +  and make further progress in the boot process.
> +
> +  @param  ImageContext           Pointer to PE/COFF image context
> +  @param  SectionHeaderOffset    Offset of PE/COFF image section header
> +  @param  NumberOfSections       Number of Sections
> +  @param  TextUpdater            Function to change code permissions
> +  @param  ReadOnlyUpdater        Function to change RO permissions
> +  @param  ReadWriteUpdater       Function to change RW permissions
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +UpdateMmFoundationPeCoffPermissions (
> +  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT      *ImageContext,
> +  IN  UINT32                                  SectionHeaderOffset,
> +  IN  CONST  UINT16                           NumberOfSections,
> +  IN  REGION_PERMISSION_UPDATE_FUNC           TextUpdater,
> +  IN  REGION_PERMISSION_UPDATE_FUNC           ReadOnlyUpdater,
> +  IN  REGION_PERMISSION_UPDATE_FUNC           ReadWriteUpdater
> +  );
> +
> +
> +/**
> +  Privileged firmware assigns RO & Executable attributes to all memory occupied
> +  by the Boot Firmware Volume. This function locates the section information of
> +  the Standalone MM Core module to be able to change permissions of the
> +  individual sections later in the boot process.
> +
> +  @param  TeData                 Pointer to PE/COFF image data
> +  @param  ImageContext           Pointer to PE/COFF image context
> +  @param  SectionHeaderOffset    Offset of PE/COFF image section header
> +  @param  NumberOfSections       Number of Sections
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetStandaloneMmCorePeCoffSections (
> +  IN        VOID                            *TeData,
> +  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT    *ImageContext,
> +  IN  OUT   UINT32                          *SectionHeaderOffset,
> +  IN  OUT   UINT16                          *NumberOfSections
> +  );
> +
> +
> +/**
> +  Privileged firmware assigns RO & Executable attributes to all memory occupied
> +  by the Boot Firmware Volume. This function locates the Standalone MM Core
> +  module PE/COFF image in the BFV and returns this information.
> +
> +  @param  BfvAddress             Base Address of Boot Firmware Volume
> +  @param  TeData                 Pointer to address for allocating memory for
> +                                 PE/COFF image data
> +  @param  TeDataSize             Pointer to size of PE/COFF image data
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +LocateStandaloneMmCorePeCoffData (
> +  IN        EFI_FIRMWARE_VOLUME_HEADER      *BfvAddress,
> +  IN  OUT   VOID                            **TeData,
> +  IN  OUT   UINTN                           *TeDataSize
> +  );
> +
> +
> +/**
> +  Use the boot information passed by privileged firmware to populate a HOB list
> +  suitable for consumption by the MM Core and drivers.
> +
> +  @param  CpuDriverEntryPoint    Address of MM CPU driver entrypoint
> +  @param  PayloadBootInfo        Boot information passed by privileged firmware
> +
> +**/
> +VOID *
> +EFIAPI
> +CreateHobListFromBootInfo (
> +  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT *CpuDriverEntryPoint,
> +  IN       EFI_SECURE_PARTITION_BOOT_INFO     *PayloadBootInfo
> +  );
> +
> +
> +/**
> +  The entry point of Standalone MM Foundation.
> +
> +  @param  SharedBufAddress  Pointer to the Buffer between SPM and SP.
> +  @param  cookie1.
> +  @param  cookie2.
> +**/
> +VOID
> +EFIAPI
> +_ModuleEntryPoint (
> +  IN VOID    *SharedBufAddress,
> +  IN UINT64  cookie1,
> +  IN UINT64  cookie2
> +  );

This API has been changed from the previous revision but the corresponding
change has not landed in Arm TF [1]. Could you please revert to original API i.e.

EFIAPI
_ModuleEntryPoint (
  IN VOID    *SharedBufAddress,
  IN UINT64  SharedBufSize,
  IN UINT64  cookie1,
  IN UINT64  cookie2
  );

[1] https://github.com/ARM-software/arm-trusted-firmware/blob/b6c07bbb2ed520f0b51775adb5d22b294d6971eb/services/std_svc/spm/sp_setup.c#L56

> +
> +
> +/**
> +  Auto generated function that calls the library constructors for all of the module's dependent libraries.
> +
> +  This function must be called by _ModuleEntryPoint().
> +  This function calls the set of library constructors for the set of library instances
> +  that a module depends on.  This includes library instances that a module depends on
> +  directly and library instances that a module depends on indirectly through other
> +  libraries. This function is auto generated by build tools and those build tools are
> +  responsible for collecting the set of library instances, determine which ones have
> +  constructors, and calling the library constructors in the proper order based upon
> +  each of the library instances own dependencies.
> +
> +  @param  ImageHandle  The image handle of the DXE Core.
> +  @param  SystemTable  A pointer to the EFI System Table.
> +
> +**/
> +VOID
> +EFIAPI
> +ProcessLibraryConstructorList (
> +  IN EFI_HANDLE             ImageHandle,
> +  IN EFI_MM_SYSTEM_TABLE  *MmSystemTable
> +  );
> +
> +
> +/**
> +  Auto generated function that calls a set of module entry points.
> +
> +  This function must be called by _ModuleEntryPoint().
> +  This function calls the set of module entry points.
> +  This function is auto generated by build tools and those build tools are responsible
> +  for collecting the module entry points and calling them in a specified order.
> +
> +  @param  HobStart  Pointer to the beginning of the HOB List passed in from the PEI Phase.
> +
> +**/
> +VOID
> +EFIAPI
> +ProcessModuleEntryPointList (
> +  IN VOID  *HobStart
> +  );
> +
> +#endif
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
> new file mode 100644
> index 0000000..d8844c5
> --- /dev/null
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
> @@ -0,0 +1,200 @@
> +/** @file
> +  Creates HOB during Standalone MM Foundation entry point
> +  on ARM platforms.
> +
> +Copyright (c) 2017 - 2018, ARM Ltd. All rights reserved.<BR>
> +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 <PiMm.h>
> +
> +#include <PiPei.h>
> +#include <Guid/MmramMemoryReserve.h>
> +#include <Guid/MpInformation.h>
> +
> +#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
> +#include <Library/ArmMmuLib.h>
> +#include <Library/ArmSvcLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/SerialPortLib.h>
> +
> +#include <IndustryStandard/ArmStdSmc.h>
> +
> +extern EFI_HOB_HANDOFF_INFO_TABLE*
> +HobConstructor (
> +  IN VOID   *EfiMemoryBegin,
> +  IN UINTN  EfiMemoryLength,
> +  IN VOID   *EfiFreeMemoryBottom,
> +  IN VOID   *EfiFreeMemoryTop
> +  );
> +
> +// GUID to identify HOB with whereabouts of communication buffer with Normal
> +// World
> +extern EFI_GUID gEfiStandaloneMmNonSecureBufferGuid;
> +
> +// GUID to identify HOB where the entry point of the CPU driver will be
> +// populated to allow this entry point driver to invoke it upon receipt of an
> +// event
> +extern EFI_GUID gEfiArmTfCpuDriverEpDescriptorGuid;
> +
> +/**
> +  Use the boot information passed by privileged firmware to populate a HOB list
> +  suitable for consumption by the MM Core and drivers.
> +
> +  @param  PayloadBootInfo    Boot information passed by privileged firmware
> +
> +**/
> +VOID *
> +CreateHobListFromBootInfo (
> +  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT *CpuDriverEntryPoint,
> +  IN       EFI_SECURE_PARTITION_BOOT_INFO     *PayloadBootInfo
> +)
> +{
> +  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;
> +  UINT32                          Index;
> +  UINT32                          BufferSize;
> +  UINT32                          Flags;
> +  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
> +  EFI_MMRAM_DESCRIPTOR            *MmramRanges;
> +  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
> +  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
> +  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;
> +  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;
> +  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *CpuDriverEntryPointDesc;
> +
> +  // Create a hoblist with a PHIT and EOH
> +  HobStart = HobConstructor ((VOID *) PayloadBootInfo->SpMemBase,
> +    (UINTN)  PayloadBootInfo->SpMemLimit - PayloadBootInfo->SpMemBase,
> +    (VOID *) PayloadBootInfo->SpHeapBase,
> +    (VOID *) (PayloadBootInfo->SpHeapBase + PayloadBootInfo->SpHeapSize));
> +
> +  // Check that the Hoblist starts at the bottom of the Heap
> +  ASSERT (HobStart == (VOID *) PayloadBootInfo->SpHeapBase);
> +
> +  // Build a Boot Firmware Volume HOB
> +  BuildFvHob(PayloadBootInfo->SpImageBase, PayloadBootInfo->SpImageSize);
> +
> +  // Build a resource descriptor Hob that describes the available physical
> +  // memory range
> +  Attributes =(
> +    EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +    EFI_RESOURCE_ATTRIBUTE_TESTED |
> +    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> +    EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> +    EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> +    EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
> +  );
> +
> +  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> +    Attributes,
> +    (UINTN) PayloadBootInfo->SpMemBase,
> +    PayloadBootInfo->SpMemLimit - PayloadBootInfo->SpMemBase);
> +
> +  // Find the size of the GUIDed HOB with MP information
> +  BufferSize = sizeof (MP_INFORMATION_HOB_DATA);
> +  BufferSize += sizeof (EFI_PROCESSOR_INFORMATION) * PayloadBootInfo->NumCpus;
> +
> +  // Create a Guided MP information HOB to enable the ARM TF CPU driver to
> +  // perform per-cpu allocations.
> +  MpInformationHobData = BuildGuidHob(&gMpInformationHobGuid, BufferSize);
> +
> +  // Populate the MP information HOB with the topology information passed by
> +  // privileged firmware
> +  MpInformationHobData->NumberOfProcessors = PayloadBootInfo->NumCpus;
> +  MpInformationHobData->NumberOfEnabledProcessors = PayloadBootInfo->NumCpus;
> +  ProcInfoBuffer = MpInformationHobData->ProcessorInfoBuffer;
> +  CpuInfo = PayloadBootInfo->CpuInfo;
> +
> +  for (Index = 0; Index < PayloadBootInfo->NumCpus; Index++) {
> +    ProcInfoBuffer[Index].ProcessorId      = CpuInfo[Index].Mpidr;
> +    ProcInfoBuffer[Index].Location.Package = GET_CLUSTER_ID(CpuInfo[Index].Mpidr);
> +    ProcInfoBuffer[Index].Location.Core    = GET_CORE_ID(CpuInfo[Index].Mpidr);
> +    ProcInfoBuffer[Index].Location.Thread  = GET_CORE_ID(CpuInfo[Index].Mpidr);
> +
> +    Flags = PROCESSOR_ENABLED_BIT | PROCESSOR_HEALTH_STATUS_BIT;
> +    if (CpuInfo[Index].Flags & CPU_INFO_FLAG_PRIMARY_CPU) {
> +      Flags |= PROCESSOR_AS_BSP_BIT;
> +    }
> +    ProcInfoBuffer[Index].StatusFlag = Flags;
> +  }
> +
> +  // Create a Guided HOB to tell the ARM TF CPU driver the location and length
> +  // of the communication buffer shared with the Normal world.
> +  NsCommBufMmramRange = (EFI_MMRAM_DESCRIPTOR *) BuildGuidHob (&gEfiStandaloneMmNonSecureBufferGuid, sizeof(EFI_MMRAM_DESCRIPTOR));
> +  NsCommBufMmramRange->PhysicalStart = PayloadBootInfo->SpNsCommBufBase;
> +  NsCommBufMmramRange->CpuStart      = PayloadBootInfo->SpNsCommBufBase;
> +  NsCommBufMmramRange->PhysicalSize  = PayloadBootInfo->SpNsCommBufSize;
> +  NsCommBufMmramRange->RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
> +
> +  // Create a Guided HOB to enable the ARM TF CPU driver to share its entry
> +  // point and populate it with the address of the shared buffer
> +  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *) BuildGuidHob (&gEfiArmTfCpuDriverEpDescriptorGuid, sizeof(ARM_TF_CPU_DRIVER_EP_DESCRIPTOR));

Shorten this line to 120 chars and have a look at others too.

> +
> +  *CpuDriverEntryPoint = NULL;
> +  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;
> +
> +  // Find the size of the GUIDed HOB with SRAM ranges
> +  BufferSize = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
> +  BufferSize += PayloadBootInfo->NumSpMemRegions *
> +    sizeof(EFI_MMRAM_DESCRIPTOR);
> +
> +  // Create a GUIDed HOB with SRAM ranges
> +  MmramRangesHob = BuildGuidHob (&gEfiMmPeiMmramMemoryReserveGuid, BufferSize);
> +
> +  // Fill up the number of MMRAM memory regions
> +  MmramRangesHob->NumberOfMmReservedRegions = PayloadBootInfo->NumSpMemRegions;
> +  // Fill up the MMRAM ranges
> +  MmramRanges = &MmramRangesHob->Descriptor[0];
> +
> +  // Base and size of memory occupied by the Standalone MM image
> +  MmramRanges[0].PhysicalStart = PayloadBootInfo->SpImageBase;
> +  MmramRanges[0].CpuStart      = PayloadBootInfo->SpImageBase;
> +  MmramRanges[0].PhysicalSize  = PayloadBootInfo->SpImageSize;
> +  MmramRanges[0].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
> +
> +  // Base and size of buffer shared with privileged Secure world software
> +  MmramRanges[1].PhysicalStart = PayloadBootInfo->SpSharedBufBase;
> +  MmramRanges[1].CpuStart      = PayloadBootInfo->SpSharedBufBase;
> +  MmramRanges[1].PhysicalSize  = PayloadBootInfo->SpPcpuSharedBufSize * PayloadBootInfo->NumCpus;
> +  MmramRanges[1].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
> +
> +  // Base and size of buffer used for synchronous communication with Normal
> +  // world software
> +  MmramRanges[2].PhysicalStart = PayloadBootInfo->SpNsCommBufBase;
> +  MmramRanges[2].CpuStart      = PayloadBootInfo->SpNsCommBufBase;
> +  MmramRanges[2].PhysicalSize  = PayloadBootInfo->SpNsCommBufSize;
> +  MmramRanges[2].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
> +
> +  // Base and size of memory allocated for stacks for all cpus
> +  MmramRanges[3].PhysicalStart = PayloadBootInfo->SpStackBase;
> +  MmramRanges[3].CpuStart      = PayloadBootInfo->SpStackBase;
> +  MmramRanges[3].PhysicalSize  = PayloadBootInfo->SpPcpuStackSize * PayloadBootInfo->NumCpus;
> +  MmramRanges[3].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
> +
> +  // Base and size of heap memory shared by all cpus
> +  MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) HobStart;
> +  MmramRanges[4].CpuStart      = (EFI_PHYSICAL_ADDRESS) HobStart;
> +  MmramRanges[4].PhysicalSize  = HobStart->EfiFreeMemoryBottom - (EFI_PHYSICAL_ADDRESS) HobStart;
> +  MmramRanges[4].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
> +
> +  // Base and size of heap memory shared by all cpus
> +  MmramRanges[5].PhysicalStart = HobStart->EfiFreeMemoryBottom;
> +  MmramRanges[5].CpuStart      = HobStart->EfiFreeMemoryBottom;
> +  MmramRanges[5].PhysicalSize  = HobStart->EfiFreeMemoryTop - HobStart->EfiFreeMemoryBottom;
> +  MmramRanges[5].RegionState   = EFI_CACHEABLE;
> +
> +  return HobStart;
> +}
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
> new file mode 100644
> index 0000000..e8310d2
> --- /dev/null
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
> @@ -0,0 +1,275 @@
> +/** @file
> +  Locate, get and update PE/COFF permissions during Standalone MM
> +  Foundation Entry point on ARM platforms.
> +
> +Copyright (c) 2017 - 2018, ARM Ltd. All rights reserved.<BR>
> +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 <PiMm.h>
> +
> +#include <PiPei.h>
> +#include <Guid/MmramMemoryReserve.h>
> +#include <Guid/MpInformation.h>
> +
> +#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
> +#include <Library/ArmMmuLib.h>
> +#include <Library/ArmSvcLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/SerialPortLib.h>
> +
> +#include <IndustryStandard/ArmStdSmc.h>
> +
> +EFI_STATUS
> +EFIAPI
> +UpdateMmFoundationPeCoffPermissions (
> +  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT      *ImageContext,
> +  IN  UINT32                                  SectionHeaderOffset,
> +  IN  CONST  UINT16                           NumberOfSections,
> +  IN  REGION_PERMISSION_UPDATE_FUNC           TextUpdater,
> +  IN  REGION_PERMISSION_UPDATE_FUNC           ReadOnlyUpdater,
> +  IN  REGION_PERMISSION_UPDATE_FUNC           ReadWriteUpdater
> +  )
> +{
> +  EFI_IMAGE_SECTION_HEADER         SectionHeader;
> +  RETURN_STATUS                    Status;
> +  EFI_PHYSICAL_ADDRESS             Base;
> +  UINTN                            Size;
> +  UINTN                            ReadSize;
> +  UINTN                            Index;
> +
> +  ASSERT (ImageContext != NULL);
> +
> +  //
> +  // Iterate over the sections
> +  //
> +  for (Index = 0; Index < NumberOfSections; Index++) {
> +    //
> +    // Read section header from file
> +    //
> +    Size = sizeof (EFI_IMAGE_SECTION_HEADER);
> +    ReadSize = Size;
> +    Status = ImageContext->ImageRead (ImageContext->Handle, SectionHeaderOffset,
> +                                   &Size, &SectionHeader);
> +    if (RETURN_ERROR (Status) || (Size != ReadSize)) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: ImageContext->ImageRead () failed (Status = %r)\n",
> +        __FUNCTION__, Status));
> +      return Status;
> +    }
> +
> +    DEBUG ((DEBUG_INFO,
> +            "%a: Section %d of image at 0x%lx has 0x%x permissions\n",
> +            __FUNCTION__, Index, ImageContext->ImageAddress, SectionHeader.Characteristics));
> +    DEBUG ((DEBUG_INFO,
> +            "%a: Section %d of image at 0x%lx has %s name\n",
> +            __FUNCTION__, Index, ImageContext->ImageAddress, SectionHeader.Name));
> +    DEBUG ((DEBUG_INFO,
> +            "%a: Section %d of image at 0x%lx has 0x%x address\n",
> +            __FUNCTION__, Index, ImageContext->ImageAddress, ImageContext->ImageAddress + SectionHeader.VirtualAddress));
> +    DEBUG ((DEBUG_INFO,
> +            "%a: Section %d of image at 0x%lx has 0x%x data\n",
> +            __FUNCTION__, Index, ImageContext->ImageAddress, SectionHeader.PointerToRawData));
> +
> +    //
> +    // If the section is marked as XN then remove the X attribute. Furthermore,
> +    // if it is a writeable section then mark it appropriately as well.
> +    //
> +    if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) {
> +      Base = ImageContext->ImageAddress + SectionHeader.VirtualAddress;
> +
> +      TextUpdater (Base, SectionHeader.Misc.VirtualSize);
> +
> +      if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) != 0) {
> +        ReadWriteUpdater (Base, SectionHeader.Misc.VirtualSize);
> +        DEBUG ((DEBUG_INFO,
> +                "%a: Mapping section %d of image at 0x%lx with RW-XN permissions\n",
> +                __FUNCTION__, Index, ImageContext->ImageAddress));
> +      } else {
> +        DEBUG ((DEBUG_INFO,
> +                "%a: Mapping section %d of image at 0x%lx with RO-XN permissions\n",
> +                __FUNCTION__, Index, ImageContext->ImageAddress));
> +      }
> +    } else {
> +        DEBUG ((DEBUG_INFO,
> +                "%a: Ignoring section %d of image at 0x%lx with 0x%x permissions\n",
> +                __FUNCTION__, Index, ImageContext->ImageAddress, SectionHeader.Characteristics));
> +    }
> +    SectionHeaderOffset += sizeof (EFI_IMAGE_SECTION_HEADER);
> +  }
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +LocateStandaloneMmCorePeCoffData (
> +  IN        EFI_FIRMWARE_VOLUME_HEADER      *BfvAddress,
> +  IN  OUT   VOID                            **TeData,
> +  IN  OUT   UINTN                           *TeDataSize
> +  )
> +{
> +  EFI_FFS_FILE_HEADER             *FileHeader = NULL;
> +  EFI_STATUS                      Status;
> +
> +  Status = FfsFindNextFile (
> +                  EFI_FV_FILETYPE_SECURITY_CORE,
> +                  BfvAddress,
> +                  &FileHeader);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM FFS file - 0x%x\n",
> +      Status));
> +    return Status;
> +  }
> +
> +  Status = FfsFindSectionData (EFI_SECTION_PE32, FileHeader, TeData, TeDataSize);
> +  if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Section data - 0x%x\n",
> +              Status));
> +    return Status;
> +  }
> +
> +  DEBUG ((DEBUG_INFO, "Found Standalone MM PE data - 0x%x\n", *TeData));
> +  return Status;
> +}
> +
> +static
> +EFI_STATUS
> +GetPeCoffSectionInformation (
> +  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT      *ImageContext,
> +  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT      *TmpContext,
> +  IN  OUT   UINT32                            *SectionHeaderOffset,
> +  IN  OUT   UINT16                            *NumberOfSections
> +  )
> +{
> +  RETURN_STATUS                         Status;
> +  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION   Hdr;
> +  EFI_IMAGE_OPTIONAL_HEADER_UNION       HdrData;
> +  UINTN                                 Size;
> +  UINTN                                 ReadSize;
> +
> +  ASSERT (ImageContext != NULL);
> +  ASSERT (TmpContext != NULL);
> +  ASSERT (SectionHeaderOffset != NULL);
> +  ASSERT (NumberOfSections != NULL);
> +
> +  //
> +  // We need to copy ImageContext since PeCoffLoaderGetImageInfo ()
> +  // will mangle the ImageAddress field
> +  //
> +  CopyMem (TmpContext, ImageContext, sizeof (*TmpContext));
> +
> +  if (TmpContext->PeCoffHeaderOffset == 0) {
> +    Status = PeCoffLoaderGetImageInfo (TmpContext);
> +    if (RETURN_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: PeCoffLoaderGetImageInfo () failed (Status = %r)\n",
> +        __FUNCTION__, Status));
> +      return Status;
> +    }
> +  }
> +
> +  if (TmpContext->IsTeImage &&
> +      TmpContext->ImageAddress == ImageContext->ImageAddress) {
> +    DEBUG ((DEBUG_INFO, "%a: ignoring XIP TE image at 0x%lx\n", __FUNCTION__,
> +      ImageContext->ImageAddress));
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  if (TmpContext->SectionAlignment < EFI_PAGE_SIZE) {
> +    //
> +    // The sections need to be at least 4 KB aligned, since that is the
> +    // granularity at which we can tighten permissions.
> +    //
> +    if (!TmpContext->IsTeImage) {
> +      DEBUG ((DEBUG_WARN,
> +        "%a: non-TE Image at 0x%lx has SectionAlignment < 4 KB (%lu)\n",
> +        __FUNCTION__, ImageContext->ImageAddress, TmpContext->SectionAlignment));
> +    }
> +    return RETURN_UNSUPPORTED;
> +  }
> +
> +  //
> +  // Read the PE/COFF Header. For PE32 (32-bit) this will read in too much
> +  // data, but that should not hurt anything. Hdr.Pe32->OptionalHeader.Magic
> +  // determines if this is a PE32 or PE32+ image. The magic is in the same
> +  // location in both images.
> +  //
> +  Hdr.Union = &HdrData;
> +  Size = sizeof (EFI_IMAGE_OPTIONAL_HEADER_UNION);
> +  ReadSize = Size;
> +  Status = TmpContext->ImageRead (TmpContext->Handle,
> +                         TmpContext->PeCoffHeaderOffset, &Size, Hdr.Pe32);
> +  if (RETURN_ERROR (Status) || (Size != ReadSize)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: TmpContext->ImageRead () failed (Status = %r)\n",
> +      __FUNCTION__, Status));
> +    return Status;
> +  }
> +
> +  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
> +
> +  *SectionHeaderOffset = TmpContext->PeCoffHeaderOffset + sizeof (UINT32) +
> +                        sizeof (EFI_IMAGE_FILE_HEADER);
> +  *NumberOfSections    = Hdr.Pe32->FileHeader.NumberOfSections;
> +
> +  switch (Hdr.Pe32->OptionalHeader.Magic) {
> +    case EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC:
> +      *SectionHeaderOffset += Hdr.Pe32->FileHeader.SizeOfOptionalHeader;
> +      break;
> +    case EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC:
> +      *SectionHeaderOffset += Hdr.Pe32Plus->FileHeader.SizeOfOptionalHeader;
> +      break;
> +    default:
> +      ASSERT (FALSE);
> +  }
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +GetStandaloneMmCorePeCoffSections (
> +  IN        VOID                            *TeData,
> +  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT    *ImageContext,
> +  IN  OUT   UINT32                          *SectionHeaderOffset,
> +  IN  OUT   UINT16                          *NumberOfSections
> +  )
> +{
> +  EFI_STATUS                   Status;
> +  PE_COFF_LOADER_IMAGE_CONTEXT TmpContext;
> +
> +  // Initialize the Image Context
> +  ZeroMem (ImageContext, sizeof (PE_COFF_LOADER_IMAGE_CONTEXT));
> +  ImageContext->Handle    = TeData;
> +  ImageContext->ImageRead = PeCoffLoaderImageReadFromMemory;
> +
> +  DEBUG((DEBUG_INFO, "Found Standalone MM PE data - 0x%x\n", TeData));
> +
> +  Status = PeCoffLoaderGetImageInfo (ImageContext);
> +  if (EFI_ERROR(Status)) {
> +    DEBUG((DEBUG_ERROR, "Unable to locate Standalone MM Core PE-COFF Image information - 0x%x\n", Status));
> +    return Status;
> +  }
> +
> +  Status = GetPeCoffSectionInformation(ImageContext, &TmpContext, SectionHeaderOffset, NumberOfSections);
> +  if (EFI_ERROR(Status)) {
> +    DEBUG((DEBUG_ERROR, "Unable to locate Standalone MM Core PE-COFF Section information - 0x%x\n", Status));
> +    return Status;
> +  }
> +
> +  DEBUG((DEBUG_INFO, "Standalone MM Core PE-COFF SectionHeaderOffset - 0x%x, NumberOfSections - %d\n", *SectionHeaderOffset, *NumberOfSections));
> +
> +  return Status;
> +}
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> new file mode 100644
> index 0000000..a212136
> --- /dev/null
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> @@ -0,0 +1,287 @@
> +/** @file
> +  Entry point to the Standalone MM Foundation when initialized during the SEC
> +  phase on ARM platforms
> +
> +Copyright (c) 2017 - 2018, ARM Ltd. All rights reserved.<BR>
> +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 <PiMm.h>
> +
> +#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
> +
> +#include <PiPei.h>
> +#include <Guid/MmramMemoryReserve.h>
> +#include <Guid/MpInformation.h>
> +
> +#include <Library/ArmMmuLib.h>
> +#include <Library/ArmSvcLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/SerialPortLib.h>
> +
> +#include <IndustryStandard/ArmStdSmc.h>
> +#include <IndustryStandard/ArmMmSvc.h>
> +
> +#define SPM_MAJOR_VER_MASK        0xFFFF0000
> +#define SPM_MINOR_VER_MASK        0x0000FFFF
> +#define SPM_MAJOR_VER_SHIFT       16
> +
> +const UINT32 SPM_MAJOR_VER = 0;
> +const UINT32 SPM_MINOR_VER = 1;
> +
> +const UINT8 BOOT_PAYLOAD_VERSION = 1;
> +
> +PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT      CpuDriverEntryPoint = NULL;
> +
> +/**
> +  Retrieve a pointer to and print the boot information passed by privileged
> +  secure firmware
> +
> +  @param  SharedBufAddress The pointer memory shared with privileged firmware
> +
> +**/
> +EFI_SECURE_PARTITION_BOOT_INFO *
> +GetAndPrintBootinformation (
> +  IN VOID                      *SharedBufAddress
> +)
> +{
> +  EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
> +  EFI_SECURE_PARTITION_CPU_INFO  *PayloadCpuInfo;
> +  UINTN                          Index;
> +
> +  PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO *) SharedBufAddress;
> +
> +  if (PayloadBootInfo->Header.Version != BOOT_PAYLOAD_VERSION)
> +  {
> +    DEBUG((DEBUG_ERROR, "Boot Information Version Mismatch. Current=0x%x, Expected=0x%x.\n", PayloadBootInfo->Header.Version, BOOT_PAYLOAD_VERSION));
> +    return NULL;
> +  }
> +  DEBUG((DEBUG_INFO, "NumSpMemRegions - 0x%x\n", PayloadBootInfo->NumSpMemRegions));
> +  DEBUG((DEBUG_INFO, "SpMemBase       - 0x%lx\n", PayloadBootInfo->SpMemBase));
> +  DEBUG((DEBUG_INFO, "SpMemLimit      - 0x%lx\n", PayloadBootInfo->SpMemLimit));
> +  DEBUG((DEBUG_INFO, "SpImageBase     - 0x%lx\n", PayloadBootInfo->SpImageBase));
> +  DEBUG((DEBUG_INFO, "SpStackBase     - 0x%lx\n", PayloadBootInfo->SpStackBase));
> +  DEBUG((DEBUG_INFO, "SpHeapBase      - 0x%lx\n", PayloadBootInfo->SpHeapBase));
> +  DEBUG((DEBUG_INFO, "SpNsCommBufBase - 0x%lx\n", PayloadBootInfo->SpNsCommBufBase));
> +  DEBUG((DEBUG_INFO, "SpSharedBufBase - 0x%lx\n", PayloadBootInfo->SpSharedBufBase));
> +
> +  DEBUG((DEBUG_INFO, "SpImageSize     - 0x%x\n", PayloadBootInfo->SpImageSize));
> +  DEBUG((DEBUG_INFO, "SpPcpuStackSize - 0x%x\n", PayloadBootInfo->SpPcpuStackSize));
> +  DEBUG((DEBUG_INFO, "SpHeapSize      - 0x%x\n", PayloadBootInfo->SpHeapSize));
> +  DEBUG((DEBUG_INFO, "SpNsCommBufSize - 0x%x\n", PayloadBootInfo->SpNsCommBufSize));
> +  DEBUG((DEBUG_INFO, "SpPcpuSharedBufSize - 0x%x\n", PayloadBootInfo->SpPcpuSharedBufSize));
> +
> +  DEBUG((DEBUG_INFO, "NumCpus         - 0x%x\n", PayloadBootInfo->NumCpus));
> +  DEBUG((DEBUG_INFO, "CpuInfo         - 0x%p\n", PayloadBootInfo->CpuInfo));
> +
> +  PayloadCpuInfo = (EFI_SECURE_PARTITION_CPU_INFO *) PayloadBootInfo->CpuInfo;

It is worth having a NULL pointer check here as well.

> +
> +  for (Index = 0; Index < PayloadBootInfo->NumCpus; Index++) {
> +    DEBUG((DEBUG_INFO, "Mpidr           - 0x%lx\n", PayloadCpuInfo[Index].Mpidr));
> +    DEBUG((DEBUG_INFO, "LinearId        - 0x%x\n", PayloadCpuInfo[Index].LinearId));
> +    DEBUG((DEBUG_INFO, "Flags           - 0x%x\n", PayloadCpuInfo[Index].Flags));
> +  }
> +
> +  return PayloadBootInfo;
> +}
> +
> +VOID
> +EFIAPI
> +DelegatedEventLoop (
> +  IN ARM_SVC_ARGS *EventCompleteSvcArgs
> +  )
> +{
> +  EFI_STATUS Status;
> +  UINTN SvcStatus;
> +
> +  while(TRUE) {
> +    ArmCallSvc(EventCompleteSvcArgs);
> +
> +    DEBUG ((DEBUG_INFO, "Received delegated event\n"));
> +    DEBUG ((DEBUG_INFO, "X0 :  0x%x\n", (UINT32) EventCompleteSvcArgs->Arg0));
> +    DEBUG ((DEBUG_INFO, "X1 :  0x%x\n", (UINT32) EventCompleteSvcArgs->Arg1));
> +    DEBUG ((DEBUG_INFO, "X2 :  0x%x\n", (UINT32) EventCompleteSvcArgs->Arg2));
> +    DEBUG ((DEBUG_INFO, "X3 :  0x%x\n", (UINT32) EventCompleteSvcArgs->Arg3));
> +
> +    Status = CpuDriverEntryPoint(
> +      EventCompleteSvcArgs->Arg0,
> +      EventCompleteSvcArgs->Arg3,
> +      EventCompleteSvcArgs->Arg1);
> +    if (EFI_ERROR(Status)) {
> +      DEBUG((DEBUG_ERROR, "Failed delegated event 0x%x, Status 0x%x\n",
> +        EventCompleteSvcArgs->Arg0, Status));
> +    }
> +
> +    switch (Status) {
> +    case EFI_SUCCESS:
> +      SvcStatus = ARM_SVC_SPM_RET_SUCCESS;
> +      break;
> +    case EFI_INVALID_PARAMETER:
> +      SvcStatus = ARM_SVC_SPM_RET_INVALID_PARAMS;
> +      break;
> +    case EFI_ACCESS_DENIED:
> +      SvcStatus = ARM_SVC_SPM_RET_DENIED;
> +      break;
> +    case EFI_OUT_OF_RESOURCES:
> +      SvcStatus = ARM_SVC_SPM_RET_NO_MEMORY;
> +      break;
> +    case EFI_UNSUPPORTED:
> +      SvcStatus = ARM_SVC_SPM_RET_NOT_SUPPORTED;
> +      break;
> +    default:
> +      SvcStatus = ARM_SVC_SPM_RET_NOT_SUPPORTED;
> +      break;
> +    }
> +
> +    EventCompleteSvcArgs->Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
> +    EventCompleteSvcArgs->Arg1 = SvcStatus;
> +  }
> +}
> +
> +STATIC
> +EFI_STATUS
> +GetSpmVersion ()
> +{
> +  EFI_STATUS   Status;
> +  UINT16       SpmMajorVersion;
> +  UINT16       SpmMinorVersion;
> +  UINT32       SpmVersion;
> +  ARM_SVC_ARGS SpmVersionArgs;
> +
> +  SpmVersionArgs.Arg0 = ARM_SVC_ID_SPM_VERSION_AARCH32;
> +
> +  ArmCallSvc (&SpmVersionArgs);
> +
> +  SpmVersion = SpmVersionArgs.Arg0;
> +
> +  SpmMajorVersion = ((SpmVersion & SPM_MAJOR_VER_MASK) >> SPM_MAJOR_VER_SHIFT);
> +  SpmMinorVersion = ((SpmVersion & SPM_MINOR_VER_MASK) >> 0);
> +
> +  // Different major revision values indicate possibly incompatible functions.
> +  // For two revisions, A and B, for which the major revision values are
> +  // identical, if the minor revision value of revision B is greater than
> +  // the minor revision value of revision A, then every function in
> +  // revision A must work in a compatible way with revision B.
> +  // However, it is possible for revision B to have a higher
> +  // function count than revision A.
> +  if ((SpmMajorVersion == SPM_MAJOR_VER) &&
> +      (SpmMinorVersion >= SPM_MINOR_VER))
> +  {
> +    DEBUG ((DEBUG_INFO, "SPM Version: Major=0x%x, Minor=0x%x\n",
> +           SpmMajorVersion, SpmMinorVersion));
> +    Status = EFI_SUCCESS;
> +  }
> +  else
> +  {
> +    DEBUG ((DEBUG_INFO, "Incompatible SPM Versions.\n Current Version: Major=0x%x, Minor=0x%x.\n Expected: Major=0x%x, Minor>=0x%x.\n",
> +            SpmMajorVersion, SpmMinorVersion, SPM_MAJOR_VER, SPM_MINOR_VER));
> +    Status = EFI_UNSUPPORTED;
> +  }
> +
> +  return Status;
> +}
> +
> +/**
> +  The entry point of Standalone MM Foundation.
> +
> +  @param  SharedBufAddress  Pointer to the Buffer between SPM and SP.
> +  @param  cookie1.
> +  @param  cookie2.
> +
> +**/
> +VOID
> +EFIAPI
> +_ModuleEntryPoint (
> +  IN VOID    *SharedBufAddress,
> +  IN UINT64  cookie1,
> +  IN UINT64  cookie2
> +  )
> +{
> +  PE_COFF_LOADER_IMAGE_CONTEXT            ImageContext;
> +  EFI_SECURE_PARTITION_BOOT_INFO          *PayloadBootInfo;
> +  ARM_SVC_ARGS                            InitMmFoundationSvcArgs = {0};
> +  EFI_STATUS                              Status;
> +  UINT32                                  SectionHeaderOffset;
> +  UINT16                                  NumberOfSections;
> +  VOID                                    *HobStart;
> +  VOID                                    *TeData;
> +  UINTN                                   TeDataSize;
> +
> +  Status = SerialPortInitialize ();
> +  ASSERT_EFI_ERROR (Status);
> +
> +  // Get Secure Partition Manager Version Information
> +  Status = GetSpmVersion();
> +  if (EFI_ERROR(Status)) {
> +    goto finish;
> +  }
> +
> +  PayloadBootInfo = GetAndPrintBootinformation(SharedBufAddress);

SharedBufAddress is dereferenced in GetAndPrintBootinformation() without a NULL
pointer check that should happen above this statement.

> +  if (PayloadBootInfo == NULL) {
> +    Status = EFI_UNSUPPORTED;
> +    goto finish;
> +  }
> +
> +  // Locate PE/COFF File information for the Standalone MM core module
> +  Status = LocateStandaloneMmCorePeCoffData (
> +                 (EFI_FIRMWARE_VOLUME_HEADER *) PayloadBootInfo->SpImageBase,
> +                 &TeData,
> +                 &TeDataSize);

Indentation of statements following LocateStandaloneMmCorePeCoffData should be fixed.

> +  if (EFI_ERROR(Status)) {
> +    goto finish;
> +  }
> +
> +  // Obtain the PE/COFF Section information for the Standalone MM core module
> +  Status = GetStandaloneMmCorePeCoffSections (
> +            TeData,
> +            &ImageContext,
> +            &SectionHeaderOffset,
> +            &NumberOfSections);

Fix indentation.

> +  if (EFI_ERROR(Status)) {
> +    goto finish;
> +  }
> +
> +  // Update the memory access permissions of individual sections in the
> +  // Standalone MM core module
> +  Status = UpdateMmFoundationPeCoffPermissions(
> +            &ImageContext,
> +            SectionHeaderOffset,
> +            NumberOfSections,
> +            ArmSetMemoryRegionNoExec,
> +            ArmSetMemoryRegionReadOnly,
> +            ArmClearMemoryRegionReadOnly);

Fix indentation.

Could you have a skim through the entire file as there a few places where the
coding convention has not been followed.

cheers,
Achin

> +  if (EFI_ERROR(Status)) {
> +    goto finish;
> +  }
> +
> +  //
> +  // Create Hoblist based upon boot information passed by privileged software
> +  //
> +  HobStart = CreateHobListFromBootInfo(&CpuDriverEntryPoint, PayloadBootInfo);
> +
> +  //
> +  // Call the MM Core entry point
> +  //
> +  ProcessModuleEntryPointList (HobStart);
> +
> +  ASSERT_EFI_ERROR (CpuDriverEntryPoint);
> +  DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP 0x%lx\n", (UINT64) CpuDriverEntryPoint));
> +
> +finish:
> +  InitMmFoundationSvcArgs.Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
> +  InitMmFoundationSvcArgs.Arg1 = Status;
> +  DelegatedEventLoop(&InitMmFoundationSvcArgs);
> +  ASSERT_EFI_ERROR(0);
> +
> +}
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> new file mode 100644
> index 0000000..66184c4
> --- /dev/null
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> @@ -0,0 +1,55 @@
> +## @file
> +# Module entry point library for DXE core.
> +#
> +# Copyright (c) 2017 - 2018, ARM Ltd. All rights reserved.<BR>
> +#
> +#  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                      = StandaloneMmCoreEntryPoint
> +  FILE_GUID                      = C97AC593-109A-4C63-905C-675FDE2689E8
> +  MODULE_TYPE                    = MM_CORE_STANDALONE
> +  VERSION_STRING                 = 1.0
> +  PI_SPECIFICATION_VERSION       = 0x00010032
> +  LIBRARY_CLASS                  = StandaloneMmCoreEntryPoint|MM_CORE_STANDALONE
> +
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC (EBC is for build only)
> +#
> +
> +[Sources.AARCH64]
> +  AArch64/StandaloneMmCoreEntryPoint.c
> +  AArch64/SetPermissions.c
> +  AArch64/CreateHobList.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  StandaloneMmPkg/StandaloneMmPkg.dec
> +
> +[Packages.AARCH64]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +
> +[LibraryClasses.AARCH64]
> +  ArmMmuLib
> +  ArmSvcLib
> +
> +[Guids]
> +  gMpInformationHobGuid
> +  gEfiMmPeiMmramMemoryReserveGuid
> +  gEfiStandaloneMmNonSecureBufferGuid
> +  gEfiArmTfCpuDriverEpDescriptorGuid
> -- 
> 2.7.4
>