Re: [PATCH] ArmPkg/ArmMmuLib: base page table VA size on GCD memory map size

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

Please disregard, I will follow up with a proper series

On 9 September 2016 at 09:07, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> As reported by Eugene, the practice of sizing the address space in the
> virtual memory system based on the maximum address in the table passed
> to ArmConfigureMmu() is problematic, since it fails to take into account
> the fact that the GCD memory space may be extended at a later time, both
> for memory and for MMIO. So instead, choose the VA size identical to the
> GCD memory map size, which is based on PcdPrePiCpuMemorySize on ARM
> systems.
>
> Reported-by: Eugene Cohen <eugene@hp.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 14 ++------------
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  4 ++++
>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  4 ++++
>  3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 6e05e6085011..b5900a761f80 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -582,9 +582,7 @@ ArmConfigureMmu (
>    VOID*                         TranslationTable;
>    UINTN                         TranslationTablePageCount;
>    UINT32                        TranslationTableAttribute;
> -  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTableEntry;
>    UINT64                        MaxAddress;
> -  UINT64                        TopAddress;
>    UINTN                         T0SZ;
>    UINTN                         RootTableEntryCount;
>    UINT64                        TCR;
> @@ -595,16 +593,8 @@ ArmConfigureMmu (
>      return RETURN_INVALID_PARAMETER;
>    }
>
> -  // Identify the highest address of the memory table
> -  MaxAddress = MemoryTable->PhysicalBase + MemoryTable->Length - 1;
> -  MemoryTableEntry = MemoryTable;
> -  while (MemoryTableEntry->Length != 0) {
> -    TopAddress = MemoryTableEntry->PhysicalBase + MemoryTableEntry->Length - 1;
> -    if (TopAddress > MaxAddress) {
> -      MaxAddress = TopAddress;
> -    }
> -    MemoryTableEntry++;
> -  }
> +  // Cover the entire GCD memory space
> +  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
>
>    // Lookup the Table Level to get the information
>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index 1533c2944e8e..b9f264de8d26 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -32,6 +32,7 @@ [Sources.ARM]
>
>  [Packages]
>    ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
>    MdePkg/MdePkg.dec
>
>  [LibraryClasses]
> @@ -39,5 +40,8 @@ [LibraryClasses]
>    CacheMaintenanceLib
>    MemoryAllocationLib
>
> +[Pcd.AARCH64]
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> +
>  [Pcd.ARM]
>    gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> index 14ebf8de673d..ecf13f790734 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -28,9 +28,13 @@ [Sources.AARCH64]
>
>  [Packages]
>    ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
>    MdePkg/MdePkg.dec
>
>  [LibraryClasses]
>    ArmLib
>    CacheMaintenanceLib
>    MemoryAllocationLib
> +
> +[Pcd.AARCH64]
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> --
> 2.7.4
>