[edk2,v1,09/21] ArmVirtualizationPkg: implement custom MemoryInitPeiLib

This implements a MemoryInitPeiLib instance that differs from the
stock ArmPlatformPkg version only in the fact that it does not remove
the memory used by the flash device (FD). The reason is that, when using
PrePi, the DXE core is started immediately and never returns so there is
no reason to preserve any of the memory that the flash device occupied
originally, and it is preferable to release is so that the OS loader
can reuse it. This is especially important for the relocatable PrePi
configuration, which is aimed at being launched from a boot loader that
itself adheres to the Linux arm64 boot protocol.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 .../ArmVirtualizationMemoryInitPeiLib.c            | 91 ++++++++++++++++++++++
 .../ArmVirtualizationMemoryInitPeiLib.inf          | 64 +++++++++++++++
 2 files changed, 155 insertions(+)
 create mode 100644 ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c
 create mode 100644 ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf

On 23 January 2015 at 20:59, Laszlo Ersek <lersek@redhat.com> wrote:
> some notes
>
> On 01/23/15 16:02, Ard Biesheuvel wrote:
>> This implements a MemoryInitPeiLib instance that differs from the
>> stock ArmPlatformPkg version only in the fact that it does not remove
>> the memory used by the flash device (FD). The reason is that, when using
>> PrePi, the DXE core is started immediately and never returns so there is
>> no reason to preserve any of the memory that the flash device occupied
>> originally, and it is preferable to release is so that the OS loader
>> can reuse it. This is especially important for the relocatable PrePi
>> configuration, which is aimed at being launched from a boot loader that
>> itself adheres to the Linux arm64 boot protocol.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  .../ArmVirtualizationMemoryInitPeiLib.c            | 91 ++++++++++++++++++++++
>>  .../ArmVirtualizationMemoryInitPeiLib.inf          | 64 +++++++++++++++
>>  2 files changed, 155 insertions(+)
>>  create mode 100644 ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c
>>  create mode 100644 ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf
>>
>> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c
>> new file mode 100644
>> index 000000000000..5f6cd059c47f
>> --- /dev/null
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c
>> @@ -0,0 +1,91 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
>> +*  Copyright (c) 2014, Linaro 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/ArmPlatformLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/HobLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/PcdLib.h>
>> +
>> +VOID
>> +BuildMemoryTypeInformationHob (
>> +  VOID
>> +  );
>> +
>> +VOID
>> +InitMmu (
>> +  VOID
>> +  )
>> +{
>> +  ARM_MEMORY_REGION_DESCRIPTOR  *MemoryTable;
>> +  VOID                          *TranslationTableBase;
>> +  UINTN                         TranslationTableSize;
>> +  RETURN_STATUS                 Status;
>> +
>> +  // Get Virtual Memory Map from the Platform Library
>> +  ArmPlatformGetVirtualMemoryMap (&MemoryTable);
>> +
>> +  //Note: Because we called PeiServicesInstallPeiMemory() before to call InitMmu() the MMU Page Table resides in
>> +  //      DRAM (even at the top of DRAM as it is the first permanent memory allocation)
>> +  Status = ArmConfigureMmu (MemoryTable, &TranslationTableBase, &TranslationTableSize);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((EFI_D_ERROR, "Error: Failed to enable MMU\n"));
>> +  }
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +MemoryPeim (
>> +  IN EFI_PHYSICAL_ADDRESS               UefiMemoryBase,
>> +  IN UINT64                             UefiMemorySize
>> +  )
>> +{
>> +  EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
>> +
>> +  // Ensure PcdSystemMemorySize has been set
>> +  ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>> +
>> +  //
>> +  // Now, the permanent memory has been installed, we can call AllocatePages()
>> +  //
>> +  ResourceAttributes = (
>> +      EFI_RESOURCE_ATTRIBUTE_PRESENT |
>> +      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
>> +      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
>> +      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,
>> +      PcdGet64 (PcdSystemMemoryBase),
>> +      PcdGet64 (PcdSystemMemorySize)
>> +  );
>> +
>> +  // Build Memory Allocation Hob
>> +  InitMmu ();
>> +
>> +  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
>> +    // Optional feature that helps prevent EFI memory map fragmentation.
>> +    BuildMemoryTypeInformationHob ();
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf
>> new file mode 100644
>> index 000000000000..1031f64fb8de
>> --- /dev/null
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf
>> @@ -0,0 +1,64 @@
>> +#/** @file
>> +#
>> +#  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
>> +#  Copyright (c) 2014, 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                    = 0x00010005
>> +  BASE_NAME                      = ArmVirtMemoryInitPeiLib
>> +  FILE_GUID                      = 021b6156-3cc8-4e99-85ee-13d8a871edf2
>> +  MODULE_TYPE                    = SEC
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = PlatformPeiLib
>> +
>> +[Sources]
>> +  ArmVirtualizationMemoryInitPeiLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  ArmPkg/ArmPkg.dec
>> +  ArmPlatformPkg/ArmPlatformPkg.dec
>> +
>> +[LibraryClasses]
>> +  DebugLib
>> +  HobLib
>> +  ArmLib
>> +  ArmPlatformLib
>> +
>> +[Guids]
>> +  gEfiMemoryTypeInformationGuid
>> +
>> +[FeaturePcd]
>> +  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
>> +
>> +[FixedPcd]
>> +  gArmTokenSpaceGuid.PcdFdBaseAddress
>> +  gArmTokenSpaceGuid.PcdFdSize
>> +
>> +  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
>> +
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode
>> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
>> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
>> +  gArmTokenSpaceGuid.PcdSystemMemorySize
>> +
>> +[Depex]
>> +  TRUE
>>
>
> It makes sense to compare the files added here with their origins:
>
>> --- MemoryInitPei/MemoryInitPeiLib.inf        2014-10-01 00:12:25.358758489 +0200
>> +++ ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.inf      2015-01-23 21:27:39.718619437 +0100
>> @@ -1,8 +1,9 @@
>>  #/** @file
>>  #
>>  #  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
>> +#  Copyright (c) 2014, 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
>>  #
>> @@ -11,19 +12,18 @@
>>  #
>>  #**/
>>
>>  [Defines]
>>    INF_VERSION                    = 0x00010005
>> -  BASE_NAME                      = ArmMemoryInitPeiLib
>> -  FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>> +  BASE_NAME                      = ArmVirtMemoryInitPeiLib
>> +  FILE_GUID                      = 021b6156-3cc8-4e99-85ee-13d8a871edf2
>
> okay, new GUID
>
>>    MODULE_TYPE                    = SEC
>>    VERSION_STRING                 = 1.0
>>    LIBRARY_CLASS                  = PlatformPeiLib
>
> Ugh, confusing library class, but it is already confusing in the original!
>
> Both should say MemoryInitPeiLib I guess.
>

Lemme fix that

>>
>>  [Sources]
>> -  MemoryInitPeiLib.c
>> -
>> +  ArmVirtualizationMemoryInitPeiLib.c
>>
>>  [Packages]
>>    MdePkg/MdePkg.dec
>>    MdeModulePkg/MdeModulePkg.dec
>>    EmbeddedPkg/EmbeddedPkg.dec
>> @@ -55,12 +55,10 @@
>>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
>>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode
>>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData
>>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode
>>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
>> -
>> -[Pcd]
>>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>>    gArmTokenSpaceGuid.PcdSystemMemorySize
>
> Care to explain this a bit? Is this related to the relocation thing? (We need FixedPCDs because thats what we can relocate?)
>

Exactly. Or PatchPcds perhaps ...

But this is not necessarily the place to put such restrictions: I will
put the FixedPcd (or PatchPcd) in the module that actually performs
the relocations, which will force the entire platform to use those.

>>
>>  [Depex]
>>    TRUE
>> --- MemoryInitPei/MemoryInitPeiLib.c  2014-11-18 19:49:09.922977213 +0100
>> +++ ArmVirtualizationPkg/Library/ArmVirtualizationMemoryInitPeiLib/ArmVirtualizationMemoryInitPeiLib.c        2015-01-23 21:27:39.717619432 +0100
>> @@ -1,8 +1,9 @@
>>  /** @file
>>  *
>>  *  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
>> +*  Copyright (c) 2014, Linaro 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
>> @@ -44,40 +45,18 @@ InitMmu (
>>    if (EFI_ERROR (Status)) {
>>      DEBUG ((EFI_D_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
>>    )
>>  {
>>    EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
>> -  UINT64                      ResourceLength;
>> -  EFI_PEI_HOB_POINTERS        NextHob;
>> -  EFI_PHYSICAL_ADDRESS        FdTop;
>> -  EFI_PHYSICAL_ADDRESS        SystemMemoryTop;
>> -  EFI_PHYSICAL_ADDRESS        ResourceTop;
>> -  BOOLEAN                     Found;
>>
>>    // Ensure PcdSystemMemorySize has been set
>>    ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>>
>>    //
>> @@ -91,79 +70,17 @@ MemoryPeim (
>>        EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
>>        EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
>>        EFI_RESOURCE_ATTRIBUTE_TESTED
>>    );
>>
>> -  // Reserved the memory space occupied by the firmware volume
>>    BuildResourceDescriptorHob (
>>        EFI_RESOURCE_SYSTEM_MEMORY,
>>        ResourceAttributes,
>>        PcdGet64 (PcdSystemMemoryBase),
>>        PcdGet64 (PcdSystemMemorySize)
>>    );
>>
>> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
>> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
>> -
>> -  // EDK2 does not have the concept of boot firmware copied into DRAM. To avoid the DXE
>> -  // core to overwrite this area we must mark the region with the attribute non-present
>> -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && (FdTop <= SystemMemoryTop)) {
>> -    Found = FALSE;
>> -
>> -    // Search for System Memory Hob that contains the firmware
>> -    NextHob.Raw = GetHobList ();
>> -    while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
>> -      if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) &&
>> -          (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor->PhysicalStart) &&
>> -          (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength))
>> -      {
>> -        ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
>> -        ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
>> -        ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + ResourceLength;
>> -
>> -        if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor->PhysicalStart) {
>> -          if (SystemMemoryTop == FdTop) {
>> -            NextHob.ResourceDescriptor->ResourceAttribute = ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
>> -          } else {
>> -            // Create the System Memory HOB for the firmware with the non-present attribute
>> -            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
>> -                                        ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
>> -                                        PcdGet64 (PcdFdBaseAddress),
>> -                                        PcdGet32 (PcdFdSize));
>> -
>> -            // Top of the FD is system memory available for UEFI
>> -            NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
>> -            NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize);
>> -          }
>> -        } else {
>> -          // Create the System Memory HOB for the firmware with the non-present attribute
>> -          BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
>> -                                      ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
>> -                                      PcdGet64 (PcdFdBaseAddress),
>> -                                      PcdGet32 (PcdFdSize));
>> -
>> -          // Update the HOB
>> -          NextHob.ResourceDescriptor->ResourceLength = PcdGet64 (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
>> -
>> -          // If there is some memory available on the top of the FD then create a HOB
>> -          if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + ResourceLength) {
>> -            // Create the System Memory HOB for the remaining region (top of the FD)
>> -            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
>> -                                        ResourceAttributes,
>> -                                        FdTop,
>> -                                        ResourceTop - FdTop);
>> -          }
>> -        }
>> -        Found = TRUE;
>> -        break;
>> -      }
>> -      NextHob.Raw = GET_NEXT_HOB (NextHob);
>> -    }
>> -
>> -    ASSERT(Found);
>> -  }
>> -
>>    // Build Memory Allocation Hob
>>    InitMmu ();
>>
>>    if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
>>      // Optional feature that helps prevent EFI memory map fragmentation.
>
> So I guess this is what you explain in the commit message. I wanted to ask why it is safe for ArmVirtualizationQemu.dsc.
>
> And then I noticed -- in ArmVirtualizationQemu.dsc we use PrePei, and here you use PrePi. Meaning, in ArmVirtualizationQemu we use
>
>   ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>
> whereas here you're cloning & modifying
>
>   ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> and the latter isn't, nor will be, used at all in ArmVirtualizationQemu, only in the new platform DSC file & corresponding FDF that you'll introduce. IOW, ArmVirtualizationQemu will not be switched over to the code you're adding here. Is that correct?
>

Correct.

I merged the 'runtime relocatable PrePi' and 'xen/arm guest' series,
and not all of the patches belonging to the former are fully relevant
to the latter.
I will try to clean up the irrelevant or confusing bits in v2.

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/