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

["Pranav Madhu" <pranav.madhu@arm.com>]

Hi Ard,

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Wednesday, June 10, 2020 11:43 PM
> To: devel@edk2.groups.io; Pranav Madhu <Pranav.Madhu@arm.com>
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/4]
> Platform/ARM/N1SDP: Add platform library implementation
>
> 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.

Ok.

>
> > +  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?

Yes, the incorrect SEC module was referenced. This will be fixed.

Thanks,
Pranav.

<...>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.