diff mbox series

[edk2,v3,08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

Message ID 20181128143357.991-9-ard.biesheuvel@linaro.org
State New
Headers show
Series Pkg: lift 40-bit IPA space limit | expand

Commit Message

Ard Biesheuvel Nov. 28, 2018, 2:33 p.m. UTC 
In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
maximum size of the identity map on the capabilities of the CPU.
Since that may exceed what is architecturally permitted when using
4 KB pages, take MAX_ADDRESS into account as well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---
 ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--
 3 files changed, 9 insertions(+), 8 deletions(-)

-- 
2.19.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Philippe Mathieu-Daudé Nov. 28, 2018, 2:46 p.m. UTC | #1 
On 28/11/18 15:33, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
> maximum size of the identity map on the capabilities of the CPU.
> Since that may exceed what is architecturally permitted when using
> 4 KB pages, take MAX_ADDRESS into account as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index b9f264de8d26..246963361e45 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -40,8 +40,5 @@ [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 ecf13f790734..f689c193b862 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -35,6 +35,3 @@ [LibraryClasses]
>    ArmLib
>    CacheMaintenanceLib
>    MemoryAllocationLib
> -
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 4b62ecb6a476..5403b8d4070e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -604,8 +604,15 @@ ArmConfigureMmu (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  // Cover the entire GCD memory space
> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
> +  //
> +  // Limit the virtual address space to what we can actually use: UEFI
> +  // mandates a 1:1 mapping, so no point in making the virtual address
> +  // space larger than the physical address space. We also have to take
> +  // into account the architectural limitations that result from UEFI's
> +  // use of 4 KB pages.
> +  //
> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
> +                    MAX_ADDRESS);
>  
>    // Lookup the Table Level to get the information
>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
>
Laszlo Ersek Nov. 28, 2018, 7:26 p.m. UTC | #2 
On 11/28/18 15:33, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the

> maximum size of the identity map on the capabilities of the CPU.

> Since that may exceed what is architecturally permitted when using

> 4 KB pages, take MAX_ADDRESS into account as well.

>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---

>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---

>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--

>  3 files changed, 9 insertions(+), 8 deletions(-)

>

> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf

> index b9f264de8d26..246963361e45 100644

> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf

> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf

> @@ -40,8 +40,5 @@ [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 ecf13f790734..f689c193b862 100644

> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf

> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf

> @@ -35,6 +35,3 @@ [LibraryClasses]

>    ArmLib

>    CacheMaintenanceLib

>    MemoryAllocationLib

> -

> -[Pcd.AARCH64]

> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize


OK

> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> index 4b62ecb6a476..5403b8d4070e 100644

> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> @@ -604,8 +604,15 @@ ArmConfigureMmu (

>      return EFI_INVALID_PARAMETER;

>    }

>

> -  // Cover the entire GCD memory space

> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;

> +  //

> +  // Limit the virtual address space to what we can actually use: UEFI

> +  // mandates a 1:1 mapping, so no point in making the virtual address

> +  // space larger than the physical address space. We also have to take

> +  // into account the architectural limitations that result from UEFI's

> +  // use of 4 KB pages.

> +  //

> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,

> +                    MAX_ADDRESS);

>

>    // Lookup the Table Level to get the information

>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);

>


This part of edk2 (or ArmVirtQemu) is where I *always* get lost. So let
me check the call tree.

  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf [ArmVirtPkg/ArmVirtQemu.dsc]
    InitializeMemory()                            [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c]
      PeiServicesInstallPeiMemory()               [...]
      // now we've got permanent PEI RAM
      MemoryPeim()                                [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c]
        InitMmu()                                 [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c]
          ArmVirtGetMemoryMap()                   [ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c]
          ArmConfigureMmu()                       [ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c]

In patch v3 04/16, we modify ArmVirtGetMemoryMap() so that the mapping
not cover the "Peripheral space after DRAM" (which is practically
"limitless"). Good.

In this patch, we have to modify ArmConfigureMmu() as well, because the
descriptor array that we pass it from ArmVirtGetMemoryMap() does not
contain all the information that is necessary for setting up "paging" in
general. Also considering mappings that could be added later during DXE,
such as MMIO ranges of platform drivers / devices, discontiguous RAM
ranges, and so on.

So we need a "maximum" here. The maximum that we choose doesn't
"seriously" translate to an increased memory footprint of paging
artifacts, *unlike* the size of the address space that we actually map
for access. Hence in patch v3 04/16, we are conservative, but in this
patch, when selecting the maximum, we go as high as we can -- either the
architecturally permitted limit (with 4KB page size), or, if the latter
is lower, the limit supported by the current CPU.

The GCD *could* go higher (even though we'd never attempt to map that in
UEFI), assuming the CPU is capable enough (using 64KB pages at OS
runtime).

This bends my brain. :) I'm now slowly comprehending your blurb. Indeed,
the rest of the series pertains to the GCD memory space map.

Nit: the second argument of the MIN() macro is not indented
idiomatically. With that fixed (no need to repost the series just
because of it):

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Nov. 29, 2018, 3:43 p.m. UTC | #3 
On Wed, Nov 28, 2018 at 03:33:49PM +0100, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the

> maximum size of the identity map on the capabilities of the CPU.

> Since that may exceed what is architecturally permitted when using

> 4 KB pages, take MAX_ADDRESS into account as well.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


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


> ---

>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---

>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---

>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--

>  3 files changed, 9 insertions(+), 8 deletions(-)

> 

> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf

> index b9f264de8d26..246963361e45 100644

> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf

> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf

> @@ -40,8 +40,5 @@ [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 ecf13f790734..f689c193b862 100644

> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf

> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf

> @@ -35,6 +35,3 @@ [LibraryClasses]

>    ArmLib

>    CacheMaintenanceLib

>    MemoryAllocationLib

> -

> -[Pcd.AARCH64]

> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize

> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> index 4b62ecb6a476..5403b8d4070e 100644

> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> @@ -604,8 +604,15 @@ ArmConfigureMmu (

>      return EFI_INVALID_PARAMETER;

>    }

>  

> -  // Cover the entire GCD memory space

> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;

> +  //

> +  // Limit the virtual address space to what we can actually use: UEFI

> +  // mandates a 1:1 mapping, so no point in making the virtual address

> +  // space larger than the physical address space. We also have to take

> +  // into account the architectural limitations that result from UEFI's

> +  // use of 4 KB pages.

> +  //

> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,

> +                    MAX_ADDRESS);

>  

>    // Lookup the Table Level to get the information

>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);

> -- 

> 2.19.1

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index b9f264de8d26..246963361e45 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -40,8 +40,5 @@  [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 ecf13f790734..f689c193b862 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
@@ -35,6 +35,3 @@  [LibraryClasses]
   ArmLib
   CacheMaintenanceLib
   MemoryAllocationLib
-
-[Pcd.AARCH64]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 4b62ecb6a476..5403b8d4070e 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -604,8 +604,15 @@  ArmConfigureMmu (
     return EFI_INVALID_PARAMETER;
   }
 
-  // Cover the entire GCD memory space
-  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
+  //
+  // Limit the virtual address space to what we can actually use: UEFI
+  // mandates a 1:1 mapping, so no point in making the virtual address
+  // space larger than the physical address space. We also have to take
+  // into account the architectural limitations that result from UEFI's
+  // use of 4 KB pages.
+  //
+  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
+                    MAX_ADDRESS);
 
   // Lookup the Table Level to get the information
   LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);