Re: [platforms: PATCH 5/8] Marvell/Armada: Add MemoryInitPeiLib that reserves secure region

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Oct 11, 2017 at 05:40:46PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The default MemoryInitPeiLib implementation insists on reserving the
> region occupied by our own FV, while this is not necessary at all (the
> compressed payload is uncompressed elsewhere, so the moment we enter
> DXE core, we don't care about the FV contents in memory)
> 
> So clone MemoryInitPeiLib and modify it to suit our needs.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Platform/Marvell/Armada/Armada.dsc.inc                                                    |   6 +-
>  Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c   | 158 ++++++++++++++++++++
>  Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf |  46 ++++++
>  Platform/Marvell/Marvell.dec                                                              |   8 +
>  4 files changed, 217 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index 56d8941..b0a8240 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -153,7 +153,7 @@
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>  
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
> -  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> +  MemoryInitPeiLib|Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>  
>  [LibraryClasses.common.DXE_CORE]
> @@ -364,6 +364,10 @@
>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
>    gArmTokenSpaceGuid.PcdArmScr|0x531
>  
> +  # Secure region reservation
> +  gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x4000000
> +  gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x0200000
> +
>    # TRNG
>    gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF2760000
>  
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
> new file mode 100644
> index 0000000..53119f4
> --- /dev/null
> +++ b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.c
> @@ -0,0 +1,158 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +*  Copyright (c) 2017, 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 <PiPei.h>
> +
> +#include <Library/ArmMmuLib.h>
> +#include <Library/ArmPlatformLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +
> +VOID
> +BuildMemoryTypeInformationHob (
> +  VOID
> +  );
> +
> +STATIC
> +VOID
> +InitMmu (
> +  IN ARM_MEMORY_REGION_DESCRIPTOR  *MemoryTable
> +  )
> +{
> +
> +  VOID                          *TranslationTableBase;
> +  UINTN                         TranslationTableSize;
> +  RETURN_STATUS                 Status;
> +
> +  Status = ArmConfigureMmu (MemoryTable,
> +                            &TranslationTableBase,
> +                            &TranslationTableSize);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Error: Failed to enable MMU\n"));
> +  }
> +}
> +
> +/*++
> +
> +Routine Description:
> +
> +
> +
> +Arguments:
> +
> +  FileHandle  - Handle of the file being invoked.
> +  PeiServices - Describes the list of possible PEI Services.
> +
> +Returns:
> +
> +  Status -  EFI_SUCCESS if the boot mode could be set
> +
> +--*/
> +EFI_STATUS
> +EFIAPI
> +MemoryPeim (
> +  IN EFI_PHYSICAL_ADDRESS               UefiMemoryBase,
> +  IN UINT64                             UefiMemorySize
> +  )
> +{
> +  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  UINT64                       ResourceLength;
> +  EFI_PEI_HOB_POINTERS         NextHob;
> +  EFI_PHYSICAL_ADDRESS         SecureTop;
> +  EFI_PHYSICAL_ADDRESS         ResourceTop;
> +
> +  // Get Virtual Memory Map from the Platform Library
> +  ArmPlatformGetVirtualMemoryMap (&MemoryTable);
> +
> +  SecureTop = (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdSecureRegionBase) +
> +              FixedPcdGet32 (PcdSecureRegionSize);
> +
> +  //
> +  // Search for System Memory Hob that covers the secure firmware,
> +  // and punch a hole in it
> +  //
> +  for (NextHob.Raw = GetHobList ();
> +       NextHob.Raw != NULL;
> +       NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR,
> +                                 NextHob.Raw)) {
> +
> +    if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) &&
> +        (FixedPcdGet64 (PcdSecureRegionBase) >= NextHob.ResourceDescriptor->PhysicalStart) &&
> +        (SecureTop <= NextHob.ResourceDescriptor->PhysicalStart +
> +                      NextHob.ResourceDescriptor->ResourceLength))
> +    {
> +      ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> +      ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> +      ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + ResourceLength;
> +
> +      if (FixedPcdGet64 (PcdSecureRegionBase) == NextHob.ResourceDescriptor->PhysicalStart) {
> +        //
> +        // This region starts right at the start of the reserved region, so we
> +        // can simply move its start pointer and reduce its length by the same
> +        // value
> +        //
> +        NextHob.ResourceDescriptor->PhysicalStart += FixedPcdGet32 (PcdSecureRegionSize);
> +        NextHob.ResourceDescriptor->ResourceLength -= FixedPcdGet32 (PcdSecureRegionSize);
> +
> +      } else if ((NextHob.ResourceDescriptor->PhysicalStart +
> +                  NextHob.ResourceDescriptor->ResourceLength) == SecureTop) {
> +
> +        //
> +        // This region ends right at the end of the reserved region, so we
> +        // can simply reduce its length by the size of the region.
> +        //
> +        NextHob.ResourceDescriptor->ResourceLength -= FixedPcdGet32 (PcdSecureRegionSize);
> +
> +      } else {
> +        //
> +        // This region covers the reserved region. So split it into two regions,
> +        // each one touching the reserved region at either end, but not covering
> +        // it.
> +        //
> +        NextHob.ResourceDescriptor->ResourceLength = FixedPcdGet64 (PcdSecureRegionBase) -
> +                                                     NextHob.ResourceDescriptor->PhysicalStart;
> +
> +        // Create the System Memory HOB for the remaining region (top of the FD)
> +        BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> +                                    ResourceAttributes,
> +                                    SecureTop,
> +                                    ResourceTop - SecureTop);
> +      }
> +
> +      //
> +      // Reserve the memory space occupied by the secure firmware
> +      //
> +      BuildResourceDescriptorHob (EFI_RESOURCE_MEMORY_RESERVED,
> +        0,
> +        FixedPcdGet64 (PcdSecureRegionBase),
> +        FixedPcdGet32 (PcdSecureRegionSize));
> +
> +      break;
> +    }
> +    NextHob.Raw = GET_NEXT_HOB (NextHob);
> +  }
> +
> +  // Build Memory Allocation Hob
> +  InitMmu (MemoryTable);
> +
> +  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
> +    // Optional feature that helps prevent EFI memory map fragmentation.
> +    BuildMemoryTypeInformationHob ();
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
> new file mode 100644
> index 0000000..ebaed01
> --- /dev/null
> +++ b/Platform/Marvell/Armada/Library/Armada70x0MemoryInitPeiLib/Armada70x0MemoryInitPeiLib.inf
> @@ -0,0 +1,46 @@
> +#/** @file
> +#
> +#  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2017, Linaro 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                    = 0x00010019
> +  BASE_NAME                      = Armada70x0MemoryInitPeiLib
> +  FILE_GUID                      = abc4e8a7-89a7-4aea-92bc-0e9421c4a473
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = MemoryInitPeiLib|SEC PEIM
> +
> +[Sources]
> +  Armada70x0MemoryInitPeiLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  ArmPlatformLib
> +  DebugLib
> +  HobLib
> +  ArmMmuLib
> +
> +[FeaturePcd]
> +  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
> +
> +[FixedPcd]
> +  gMarvellTokenSpaceGuid.PcdSecureRegionBase
> +  gMarvellTokenSpaceGuid.PcdSecureRegionSize
> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
> index db1c7fa..63ea071 100644
> --- a/Platform/Marvell/Marvell.dec
> +++ b/Platform/Marvell/Marvell.dec
> @@ -207,6 +207,14 @@
>    gMarvellTokenSpaceGuid.PcdDramRemapSize|0x40000000|UINT32|0x50000004
>    gMarvellTokenSpaceGuid.PcdDramRemapTarget|0xC0000000|UINT32|0x50000003
>  
> +  #
> +  # The secure firmware may occupy a DRAM region that is accessible by the
> +  # normal world. These PCDs describe such a region, which will be converted
> +  # to 'reserved' memory before DXE is entered.
> +  #
> +  gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x0|UINT64|0x50000000
> +  gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x0|UINT32|0x50000001
> +
>  [Protocols]
>    gMarvellEepromProtocolGuid               = { 0x71954bda, 0x60d3, 0x4ef8, { 0x8e, 0x3c, 0x0e, 0x33, 0x9f, 0x3b, 0xc2, 0x2b }}
>    gMarvellMdioProtocolGuid                 = { 0x40010b03, 0x5f08, 0x496a, { 0xa2, 0x64, 0x10, 0x5e, 0x72, 0xd3, 0x71, 0xaa }}
> -- 
> 2.7.4
>