Re: [edk2-devel] [edk2-platforms][PATCH v1 1/4] Platform/ARM/N1SDP: Add platform library implementation

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 4/24/20 1:58 PM, Pranav Madhu via groups.io wrote:
> From: Deepak Pandey <deepak.pandey@arm.com>
> 
> N1SDP (Neoverse N1 System Development Platform) platform library support.
> This includes the virtual memory map and helper functions for platform
> intialization.
> 
> Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
> ---
>   Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.inf  |  52 ++++++++
>   Platform/ARM/N1SdpPkg/Include/N1SdpPlatform.h              |  68 +++++++++++
>   Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.c    |  67 +++++++++++
>   Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLibMem.c | 127 ++++++++++++++++++++
>   Platform/ARM/N1SdpPkg/Library/PlatformLib/AArch64/Helper.S |  84 +++++++++++++
>   5 files changed, 398 insertions(+)
> 
> diff --git a/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.inf
> new file mode 100644
> index 000000000000..1ba29657cbf6
> --- /dev/null
> +++ b/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.inf
> @@ -0,0 +1,52 @@
> +## @file
> +#
> +#  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = ArmN1SdpLib
> +  FILE_GUID                      = 87c525cd-e1a2-469e-994c-c28cd0c7bd0d

Please generate fresh GUIDs instead of reusing existing ones.

> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmPlatformLib
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/ARM/N1SdpPkg/N1SdpPlatform.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  DebugLib
> +  HobLib
> +  IoLib
> +  MemoryAllocationLib
> +  SerialPortLib
> +
> +[Sources.common]
> +  PlatformLibMem.c
> +  PlatformLib.c
> +
> +[Sources.AARCH64]
> +  AArch64/Helper.S | GCC
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> +  gArmTokenSpaceGuid.PcdArmPrimaryCore
> +  gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
> +
> +  gArmN1SdpTokenSpaceGuid.PcdDramBlock2Base
> +
> +[Guids]
> +  gEfiHobListGuid          ## CONSUMES  ## SystemTable
> +
> +[Ppis]
> +  gArmMpCoreInfoPpiGuid
> diff --git a/Platform/ARM/N1SdpPkg/Include/N1SdpPlatform.h b/Platform/ARM/N1SdpPkg/Include/N1SdpPlatform.h
> new file mode 100644
> index 000000000000..9ab4617c3c90
> --- /dev/null
> +++ b/Platform/ARM/N1SdpPkg/Include/N1SdpPlatform.h
> @@ -0,0 +1,68 @@
> +/** @file
> +*
> +* Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +*
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#ifndef __N1SDP_PLATFORM_H__
> +#define __N1SDP_PLATFORM_H__
> +
> +#define N1SDP_DRAM_BLOCK1_SIZE               SIZE_2GB
> +
> +//******************************************************************************
> +// Platform Memory Map
> +//******************************************************************************
> +
> +// SubSystem Peripherals - UART0
> +#define N1SDP_UART0_BASE                     0x2A400000
> +#define N1SDP_UART0_SZ                       SIZE_64KB
> +
> +// SubSystem Peripherals - UART1
> +#define N1SDP_UART1_BASE                     0x2A410000
> +#define N1SDP_UART1_SZ                       SIZE_64KB
> +
> +// SubSystem Peripherals - Generic Watchdog
> +#define N1SDP_GENERIC_WDOG_BASE              0x2A440000
> +#define N1SDP_GENERIC_WDOG_SZ                SIZE_128KB
> +
> +// SubSystem Peripherals - GIC(600)
> +#define N1SDP_GIC_BASE                       0x30000000
> +#define N1SDP_GICR_BASE                      0x300C0000
> +#define N1SDP_GIC_SZ                         SIZE_256KB
> +#define N1SDP_GICR_SZ                        SIZE_1MB
> +
> +// SubSystem non-secure SRAM
> +#define N1SDP_NON_SECURE_SRAM_BASE           0x06000000
> +#define N1SDP_NON_SECURE_SRAM_SZ             SIZE_64KB
> +
> +// AXI Expansion peripherals
> +#define N1SDP_EXP_PERIPH_BASE0               0x1C000000
> +#define N1SDP_EXP_PERIPH_BASE0_SZ            0x1300000
> +
> +// Base address to a structure of type N1SDP_PLAT_INFO which is pre-populated
> +// by a earlier boot stage
> +#define N1SDP_PLAT_INFO_STRUCT_BASE          (N1SDP_NON_SECURE_SRAM_BASE + \
> +                                              0x00008000)
> +
> +/*
> + * Platform information structure stored in non secure SRAM
> + * Platform information are passed from the trusted firmware with the below
> + * structure format. The elements of N1SDP_PLAT_INFO should be always in sync
> + * with the structure in trusted firmware
> + */
> +#pragma pack(1)
> +typedef struct {
> +  /*! 0 - Single Chip, 1 - Chip to Chip (C2C) */
> +  BOOLEAN MultichipMode;
> +  /*! Slave count in C2C mode */
> +  UINT8   SlaveCount;
> +  /*! Local DDR memory size in GigaBytes */
> +  UINT8   LocalDdrSize;
> +  /*! Remote DDR memory size in GigaBytes */
> +  UINT8   RemoteDdrSize;
> +} N1SDP_PLAT_INFO;
> +#pragma pack()
> +
> +#endif
> diff --git a/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.c
> new file mode 100644
> index 000000000000..e9efd7a2d5c2
> --- /dev/null
> +++ b/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLib.c
> @@ -0,0 +1,67 @@
> +/** @file
> +*
> +*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <Library/ArmPlatformLib.h>
> +#include <Library/BaseLib.h>
> +#include <Ppi/ArmMpCoreInfo.h>
> +
> +STATIC ARM_CORE_INFO mCoreInfoTable[] = {
> +  { 0x0, 0x0 }, // Cluster 0, Core 0
> +  { 0x0, 0x1 }, // Cluster 0, Core 1
> +  { 0x1, 0x0 }, // Cluster 1, Core 0
> +  { 0x1, 0x1 }  // Cluster 1, Core 1
> +};
> +
> +EFI_BOOT_MODE
> +ArmPlatformGetBootMode (
> +  VOID
> +  )
> +{
> +  return BOOT_WITH_FULL_CONFIGURATION;
> +}
> +
> +RETURN_STATUS
> +ArmPlatformInitialize (
> +  IN UINTN                      MpId
> +  )
> +{
> +  return RETURN_SUCCESS;
> +}
> +
> +EFI_STATUS
> +PrePeiCoreGetMpCoreInfo (
> +  OUT UINTN                     *CoreCount,
> +  OUT ARM_CORE_INFO             **ArmCoreTable
> +  )
> +{
> +  *CoreCount    = sizeof (mCoreInfoTable) / sizeof (ARM_CORE_INFO);
> +  *ArmCoreTable = mCoreInfoTable;
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = {
> +  PrePeiCoreGetMpCoreInfo
> +};
> +

Do you need this mpcore stuff? You are using the unicore SEC module, no?

> +EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
> +  {
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
> +    &gArmMpCoreInfoPpiGuid,
> +    &mMpCoreInfoPpi
> +  }
> +};
> +
> +VOID
> +ArmPlatformGetPlatformPpiList (
> +  OUT UINTN                     *PpiListSize,
> +  OUT EFI_PEI_PPI_DESCRIPTOR    **PpiList
> +  )
> +{
> +  *PpiListSize = sizeof (gPlatformPpiTable);
> +  *PpiList = gPlatformPpiTable;
> +}
> diff --git a/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLibMem.c b/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLibMem.c
> new file mode 100644
> index 000000000000..7fae090cdb8d
> --- /dev/null
> +++ b/Platform/ARM/N1SdpPkg/Library/PlatformLib/PlatformLibMem.c
> @@ -0,0 +1,127 @@
> +/** @file
> +*
> +*  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <Library/ArmPlatformLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <N1SdpPlatform.h>
> +
> +// The total number of descriptors, including the final "end-of-table" descriptor.
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 9
> +
> +/**
> +  Returns the Virtual Memory Map of the platform.
> +
> +  This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU
> +  on your platform.
> +
> +  @param[out] VirtualMemoryMap Array of ARM_MEMORY_REGION_DESCRIPTOR describing
> +                               a Physical-to-Virtual Memory mapping. This array
> +                               must be ended by a zero-filled entry.
> +**/
> +VOID
> +ArmPlatformGetVirtualMemoryMap (
> +  IN ARM_MEMORY_REGION_DESCRIPTOR **VirtualMemoryMap
> +  )
> +{
> +  UINTN                         Index = 0;
> +  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
> +  N1SDP_PLAT_INFO               *PlatInfo;
> +  UINT64                        DramBlock2Size;
> +
> +  PlatInfo = (N1SDP_PLAT_INFO *)N1SDP_PLAT_INFO_STRUCT_BASE;
> +  DramBlock2Size = ((UINT64)(PlatInfo->LocalDdrSize -
> +                    (N1SDP_DRAM_BLOCK1_SIZE / SIZE_1GB)) * (UINT64)SIZE_1GB);
> +
> +  ResourceAttributes =
> +    EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +    EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> +    EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> +    EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> +    EFI_RESOURCE_ATTRIBUTE_TESTED;
> +
> +  BuildResourceDescriptorHob (
> +    EFI_RESOURCE_SYSTEM_MEMORY,
> +    ResourceAttributes,
> +    FixedPcdGet64 (PcdDramBlock2Base),
> +    DramBlock2Size);
> +
> +  ASSERT (VirtualMemoryMap != NULL);
> +  Index = 0;
> +
> +  VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR *)AllocatePages
> +                       (EFI_SIZE_TO_PAGES (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> +                        MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
> +  if (VirtualMemoryTable == NULL) {
> +    return;
> +  }
> +
> +  // SubSystem Peripherals - Generic Watchdog
> +  VirtualMemoryTable[Index].PhysicalBase    = N1SDP_GENERIC_WDOG_BASE;
> +  VirtualMemoryTable[Index].VirtualBase     = N1SDP_GENERIC_WDOG_BASE;
> +  VirtualMemoryTable[Index].Length          = N1SDP_GENERIC_WDOG_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // SubSystem Peripherals - GIC-600
> +  VirtualMemoryTable[++Index].PhysicalBase  = N1SDP_GIC_BASE;
> +  VirtualMemoryTable[Index].VirtualBase     = N1SDP_GIC_BASE;
> +  VirtualMemoryTable[Index].Length          = N1SDP_GIC_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // SubSystem Peripherals - GICR-600
> +  VirtualMemoryTable[++Index].PhysicalBase  = N1SDP_GICR_BASE;
> +  VirtualMemoryTable[Index].VirtualBase     = N1SDP_GICR_BASE;
> +  VirtualMemoryTable[Index].Length          = N1SDP_GICR_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // OnChip non-secure SRAM
> +  VirtualMemoryTable[++Index].PhysicalBase  = N1SDP_NON_SECURE_SRAM_BASE;
> +  VirtualMemoryTable[Index].VirtualBase     = N1SDP_NON_SECURE_SRAM_BASE;
> +  VirtualMemoryTable[Index].Length          = N1SDP_NON_SECURE_SRAM_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
> +
> +  // SubSystem Pheripherals - UART0
> +  VirtualMemoryTable[++Index].PhysicalBase  = N1SDP_UART0_BASE;
> +  VirtualMemoryTable[Index].VirtualBase     = N1SDP_UART0_BASE;
> +  VirtualMemoryTable[Index].Length          = N1SDP_UART0_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // DDR Primary (2GB)
> +  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdSystemMemoryBase);
> +  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdSystemMemoryBase);
> +  VirtualMemoryTable[Index].Length          = PcdGet64 (PcdSystemMemorySize);
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // DDR Secondary
> +  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdDramBlock2Base);
> +  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdDramBlock2Base);
> +  VirtualMemoryTable[Index].Length          = DramBlock2Size;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // Expansion Peripherals
> +  VirtualMemoryTable[++Index].PhysicalBase  = N1SDP_EXP_PERIPH_BASE0;
> +  VirtualMemoryTable[Index].VirtualBase     = N1SDP_EXP_PERIPH_BASE0;
> +  VirtualMemoryTable[Index].Length          = N1SDP_EXP_PERIPH_BASE0_SZ;
> +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // End of Table
> +  VirtualMemoryTable[++Index].PhysicalBase  = 0;
> +  VirtualMemoryTable[Index].VirtualBase     = 0;
> +  VirtualMemoryTable[Index].Length          = 0;
> +  VirtualMemoryTable[Index].Attributes      = (ARM_MEMORY_REGION_ATTRIBUTES)0;
> +
> +  ASSERT((Index) < MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> +  DEBUG ((DEBUG_INIT, "Virtual Memory Table setup complete.\n"));
> +
> +  *VirtualMemoryMap = VirtualMemoryTable;
> +}
> diff --git a/Platform/ARM/N1SdpPkg/Library/PlatformLib/AArch64/Helper.S b/Platform/ARM/N1SdpPkg/Library/PlatformLib/AArch64/Helper.S
> new file mode 100644
> index 000000000000..8d2069dea837
> --- /dev/null
> +++ b/Platform/ARM/N1SdpPkg/Library/PlatformLib/AArch64/Helper.S
> @@ -0,0 +1,84 @@
> +/** @file
> +*
> +*  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <AsmMacroIoLibV8.h>
> +#include <Library/ArmLib.h>
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> +GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> +GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> +GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> +
> +//
> +// 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.
> +//
> +ASM_PFX(ArmPlatformPeiBootAction):
> +  ret
> +
> +//
> +// Return the core position from the value of its MpId register
> +//
> +// This function returns core position from the position 0 in the processor.
> +// This function might be called from assembler before any stack is set.
> +//
> +// @return   Return the core position
> +//
> +//UINTN
> +//ArmPlatformGetCorePosition (
> +//  IN UINTN MpId
> +//  );
> +// With this function: CorePos = (ClusterId * 2) + CoreId
> +ASM_PFX(ArmPlatformGetCorePosition):
> +  and   x1, x0, #ARM_CORE_MASK
> +  and   x0, x0, #ARM_CLUSTER_MASK
> +  add   x0, x1, x0, LSR #7
> +  ret
> +
> +//
> +// Return the MpId of the primary core
> +//
> +// This function returns the MpId of the primary core.
> +// This function might be called from assembler before any stack is set.
> +//
> +// @return   Return the MpId of the primary core
> +//
> +//UINTN
> +//ArmPlatformGetPrimaryCoreMpId (
> +//  VOID
> +//  );
> +ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
> +  MOV32 (w0, FixedPcdGet32(PcdArmPrimaryCore))
> +  ret
> +
> +//
> +// Return a non-zero value if the callee is the primary core
> +//
> +// This function returns a non-zero value if the callee is the primary core.
> +// Primary core is the core responsible to initialize hardware and run UEFI.
> +// This function might be called from assembler before any stack is set.
> +//
> +//  @return   Return a non-zero value if the callee is the primary core.
> +//
> +//UINTN
> +//ArmPlatformIsPrimaryCore (
> +//  IN UINTN MpId
> +//  );
> +ASM_PFX(ArmPlatformIsPrimaryCore):
> +  MOV32 (w1, FixedPcdGet32(PcdArmPrimaryCoreMask))
> +  and   x0, x0, x1
> +  MOV32 (w1, FixedPcdGet32(PcdArmPrimaryCore))
> +  cmp   w0, w1
> +  cset  x0, eq
> +  ret
>