Re: [edk2-platforms][PATCH V1 2/3] Platform/Sgi: add StandaloneMM usable NorFlashPlatformLib

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Sayanta,

I have a minor suggestion maked inline as [SAMI].

Otherwise this patch looks good to me.

With that addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 24/05/2021 06:22 PM, Sayanta Pattanayak wrote:
> Add the NorFlashPlatformLib library instance that can be linked with
> MM_STANDALONE modules that implement a secure variable storage. The
> third instance of the NOR flash is used as the non-volatile storage.
>
> Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak@arm.com>
> ---
>   Platform/ARM/SgiPkg/SgiPlatform.dec                                 |  1 +
>   Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf | 33 ++++++++
>   Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c   | 82 ++++++++++++++++++++
>   3 files changed, 116 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> index 3effd49592ea..af08ed153eae 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -54,6 +54,7 @@
>   
>     gArmSgiTokenSpaceGuid.PcdSmcCs0Base|0|UINT64|0x0000000C
>     gArmSgiTokenSpaceGuid.PcdSmcCs1Base|0|UINT64|0x0000000D
> +  gArmSgiTokenSpaceGuid.PcdSmcCs2Base|0|UINT64|0x00001000
>     gArmSgiTokenSpaceGuid.PcdSysPeriphBase|0x00000000|UINT64|0x0000000E
>     gArmSgiTokenSpaceGuid.PcdSysPeriphSysRegBase|0x0|UINT64|0x0000000F
>   
> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> new file mode 100644
> index 000000000000..96bbf1e42313
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> @@ -0,0 +1,33 @@
> +## @file
> +#  StandaloneMM instance of NOR Flash library.
> +#
> +#  Copyright (c) 2021, ARM Limited. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = NorFlashMmLib
> +  FILE_GUID                      = 2ce22190-b933-4d1e-99ba-8bf1f0768255
> +  MODULE_TYPE                    = MM_STANDALONE
> +  VERSION_STRING                 = 1.0
> +  PI_SPECIFICATION_VERSION       = 0x00010032
> +  LIBRARY_CLASS                  = NorFlashPlatformLib
> +
> +[Sources.common]
> +  StandaloneMmNorFlashLib.c
> +
> +[Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  IoLib
> +
> +[FixedPcd]
> +  gArmSgiTokenSpaceGuid.PcdSysPeriphSysRegBase
> +  gArmSgiTokenSpaceGuid.PcdSmcCs2Base
> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
> new file mode 100644
> index 000000000000..3e5a5612c17e
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
> @@ -0,0 +1,82 @@
> +/** @file
> +* NOR flash platform library to be used in StandaloneMM context
> +*
> +* This file provides platform callbacks for the NOR flash module that executes
> +* in the StandaloneMM context. The third NOR flash instance of 64MB size on the
> +* reference design platform is assigned to be used in the StandaloneMM context.
> +*
> +* Copyright (c) 2021, ARM Ltd. All rights reserved.
> +*
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/NorFlashPlatformLib.h>
> +#include <PiMm.h>
> +#include <SgiPlatform.h>
> +
> +//
> +// 64MB NOR flash connected to CS2 is assigned to be used in StandaloneMM
> +// context.
> +//
> +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = {
[SAMI] Minor - Can we add the CONST qualifier?
> +  {
> +    // NOR-Flash2 assigned for secure storage.
> +    FixedPcdGet64 (PcdSmcCs2Base),
> +    FixedPcdGet64 (PcdSmcCs2Base),
> +    SIZE_256KB * 256,
> +    SIZE_256KB,
> +  },
> +};
> +
> +/** Allow access to NOR flash
> +
> +  On the reference design platforms, the access to NOR flash has to be
> +  explicitly permitted by writing to the FLASH_RWEN bit of the SYSPH_SYS_REG
> +  register.
> +
> +  @retval  EFI_SUCCESS  Initialize required to access NOR flash is complete.
> +
> +**/
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  )
> +{
> +  UINT64 SysRegFlash;
> +
> +  SysRegFlash = FixedPcdGet64 (PcdSysPeriphSysRegBase) + SGI_SYSPH_SYS_REG_FLASH;
> +  MmioOr32 (SysRegFlash, SGI_SYSPH_SYS_REG_FLASH_RWEN);
> +  return EFI_SUCCESS;
> +}
> +
> +/** Returns the list of available NOR flash devices
> +
> +  For the StandaloneMM execution context, return the list of available NOR
> +  flash devices that are available for use.
> +
> +  @param[in]   NorFlashDevices  Pointer to array of NOR flash devices.
> +  @param[in]   Count            Number of elements in the NOR flash devices
> +                                array.
> +
> +  @retval  EFI_SUCCESS            Valid set of NOR flash devices is returned.
> +  @retval  EFI_INVALID_PARAMETER  Pointers to NOR flash devices and/or count is
> +                                  invalid.
> +
> +**/
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION   **NorFlashDevices,
> +  OUT UINT32                  *Count
> +  )
> +{
> +  if ((NorFlashDevices == NULL) || (Count == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *NorFlashDevices = mNorFlashDevices;
> +  *Count = ARRAY_SIZE (mNorFlashDevices);
> +  return EFI_SUCCESS;
> +}