* [PATCH] ArmPkg/ArmMmuLib: base page table VA size on GCD memory map size
@ 2016-09-09 8:07 Ard Biesheuvel
2016-09-09 10:41 ` Ard Biesheuvel
0 siblings, 1 reply; 2+ messages in thread
From: Ard Biesheuvel @ 2016-09-09 8:07 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, eugene; +Cc: Ard Biesheuvel
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ArmPkg/ArmMmuLib: base page table VA size on GCD memory map size
2016-09-09 8:07 [PATCH] ArmPkg/ArmMmuLib: base page table VA size on GCD memory map size Ard Biesheuvel
@ 2016-09-09 10:41 ` Ard Biesheuvel
0 siblings, 0 replies; 2+ messages in thread
From: Ard Biesheuvel @ 2016-09-09 10:41 UTC (permalink / raw)
To: edk2-devel-01, Leif Lindholm, Cohen, Eugene; +Cc: Ard Biesheuvel
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-09-09 10:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-09 8:07 [PATCH] ArmPkg/ArmMmuLib: base page table VA size on GCD memory map size Ard Biesheuvel
2016-09-09 10:41 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox