* [PATCH v2 1/9] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-05 16:23 ` Leif Lindholm
2020-03-04 18:12 ` [PATCH v2 2/9] ArmPkg/ArmMmuLib ARM: remove dummy constructor Ard Biesheuvel
` (8 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
Cache maintenance operations by set/way are only intended to be used
in the context of on/offlining a core, while it has been taken out of
the coherency domain. Any use intended to ensure that the contents of
the cache have made it to main memory is unreliable, since cacheline
migration and non-architected system caches may cause these contents
to linger elsewhere, without being visible in main memory once the
MMU and caches are disabled.
In KVM on Linux, there are horrid hacks in place to ensure that such
set/way operations are trapped, and replaced with a single by-VA
clean/invalidate of the entire guest VA space once the MMU state
changes, which can be costly, and is unnecessary if we manage the
caches a bit more carefully, and perform maintenance by virtual
address only.
So let's get rid of the call to ArmInvalidateDataCache () in the
PrePeiCore startup code, and instead, invalidate the UEFI memory
region by virtual address, which is the only memory region we will
be touching with the caches and MMU both disabled and enabled.
(This will lead to data corruption if data written with the MMU off
is shadowed by clean, stale cachelines that stick around when the
MMU is enabled again.)
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Tested-By: Pete Batard <pete@akeo.ie>
---
ArmPlatformPkg/PrePi/PeiMPCore.inf | 1 +
ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 +
ArmPlatformPkg/PrePi/PrePi.c | 8 +++++---
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
index 9c5da0d42a7b..053f9fd9e616 100644
--- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
@@ -37,6 +37,7 @@ [Packages]
[LibraryClasses]
BaseLib
+ CacheMaintenanceLib
DebugLib
DebugAgentLib
ArmLib
diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
index ee9b05b25337..78d218ae09ca 100644
--- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
@@ -37,6 +37,7 @@ [Packages]
[LibraryClasses]
BaseLib
+ CacheMaintenanceLib
DebugLib
DebugAgentLib
ArmLib
diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index 2bb144958139..254fb331733e 100644
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -8,6 +8,7 @@
#include <PiPei.h>
+#include <Library/CacheMaintenanceLib.h>
#include <Library/DebugAgentLib.h>
#include <Library/PrePiLib.h>
#include <Library/PrintLib.h>
@@ -178,8 +179,6 @@ CEntryPoint (
// Data Cache enabled on Primary core when MMU is enabled.
ArmDisableDataCache ();
- // Invalidate Data cache
- ArmInvalidateDataCache ();
// Invalidate instruction cache
ArmInvalidateInstructionCache ();
// Enable Instruction Caches on all cores.
@@ -200,6 +199,10 @@ CEntryPoint (
// If not primary Jump to Secondary Main
if (ArmPlatformIsPrimaryCore (MpId)) {
+
+ InvalidateDataCacheRange ((VOID *)UefiMemoryBase,
+ FixedPcdGet32(PcdSystemMemoryUefiRegionSize));
+
// Goto primary Main.
PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
} else {
@@ -209,4 +212,3 @@ CEntryPoint (
// DXE Core should always load and never return
ASSERT (FALSE);
}
-
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/9] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones
2020-03-04 18:12 ` [PATCH v2 1/9] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones Ard Biesheuvel
@ 2020-03-05 16:23 ` Leif Lindholm
0 siblings, 0 replies; 13+ messages in thread
From: Leif Lindholm @ 2020-03-05 16:23 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel
On Wed, Mar 04, 2020 at 19:12:38 +0100, Ard Biesheuvel wrote:
> Cache maintenance operations by set/way are only intended to be used
> in the context of on/offlining a core, while it has been taken out of
> the coherency domain. Any use intended to ensure that the contents of
> the cache have made it to main memory is unreliable, since cacheline
> migration and non-architected system caches may cause these contents
> to linger elsewhere, without being visible in main memory once the
> MMU and caches are disabled.
>
> In KVM on Linux, there are horrid hacks in place to ensure that such
> set/way operations are trapped, and replaced with a single by-VA
> clean/invalidate of the entire guest VA space once the MMU state
> changes, which can be costly, and is unnecessary if we manage the
> caches a bit more carefully, and perform maintenance by virtual
> address only.
>
> So let's get rid of the call to ArmInvalidateDataCache () in the
> PrePeiCore startup code, and instead, invalidate the UEFI memory
> region by virtual address, which is the only memory region we will
> be touching with the caches and MMU both disabled and enabled.
> (This will lead to data corruption if data written with the MMU off
> is shadowed by clean, stale cachelines that stick around when the
> MMU is enabled again.)
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Tested-By: Pete Batard <pete@akeo.ie>
> ---
> ArmPlatformPkg/PrePi/PeiMPCore.inf | 1 +
> ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 +
> ArmPlatformPkg/PrePi/PrePi.c | 8 +++++---
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> index 9c5da0d42a7b..053f9fd9e616 100644
> --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> @@ -37,6 +37,7 @@ [Packages]
>
> [LibraryClasses]
> BaseLib
> + CacheMaintenanceLib
> DebugLib
> DebugAgentLib
> ArmLib
> diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> index ee9b05b25337..78d218ae09ca 100644
> --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> @@ -37,6 +37,7 @@ [Packages]
>
> [LibraryClasses]
> BaseLib
> + CacheMaintenanceLib
> DebugLib
> DebugAgentLib
> ArmLib
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index 2bb144958139..254fb331733e 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -8,6 +8,7 @@
>
> #include <PiPei.h>
>
> +#include <Library/CacheMaintenanceLib.h>
> #include <Library/DebugAgentLib.h>
> #include <Library/PrePiLib.h>
> #include <Library/PrintLib.h>
> @@ -178,8 +179,6 @@ CEntryPoint (
>
> // Data Cache enabled on Primary core when MMU is enabled.
> ArmDisableDataCache ();
> - // Invalidate Data cache
> - ArmInvalidateDataCache ();
> // Invalidate instruction cache
> ArmInvalidateInstructionCache ();
> // Enable Instruction Caches on all cores.
> @@ -200,6 +199,10 @@ CEntryPoint (
>
> // If not primary Jump to Secondary Main
> if (ArmPlatformIsPrimaryCore (MpId)) {
> +
> + InvalidateDataCacheRange ((VOID *)UefiMemoryBase,
> + FixedPcdGet32(PcdSystemMemoryUefiRegionSize));
Space before that (.
With that:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> +
> // Goto primary Main.
> PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
> } else {
> @@ -209,4 +212,3 @@ CEntryPoint (
> // DXE Core should always load and never return
> ASSERT (FALSE);
> }
> -
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/9] ArmPkg/ArmMmuLib ARM: remove dummy constructor
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 1/9] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 3/9] ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code Ard Biesheuvel
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
Make the CONSTRUCTOR define in the .INF AARCH64 only, so we can drop
the empty stub that exists for ARM.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 9 ---------
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 ++
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 74ac31de98cc..a6601258bee0 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -830,12 +830,3 @@ ArmClearMemoryRegionReadOnly (
{
return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
}
-
-RETURN_STATUS
-EFIAPI
-ArmMmuBaseLibConstructor (
- VOID
- )
-{
- return RETURN_SUCCESS;
-}
diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index 5028a955afac..3dfe68ba48a6 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -14,6 +14,8 @@ [Defines]
MODULE_TYPE = BASE
VERSION_STRING = 1.0
LIBRARY_CLASS = ArmMmuLib
+
+[Defines.AARCH64]
CONSTRUCTOR = ArmMmuBaseLibConstructor
[Sources.AARCH64]
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/9] ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 1/9] ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 2/9] ArmPkg/ArmMmuLib ARM: remove dummy constructor Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 4/9] ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries Ard Biesheuvel
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
Unlike the AArch64 implementation of ArmMmuLib, which combines the
initial page table population code with the code that runs at later
stages to manage permission attributes in the page tables, ARM uses
two completely separate sets of routines for this.
Since ArmMmuLib is a static library, we can prevent duplication of
this code between different users, which usually only need one or
the other. (Note that LTO should also achieve the same.)
This also makes it easier to reason about modifying the cache
maintenance handling, and replace the set/way ops with by-VA
ops, since the code that performs the set/way ops only executes
when the MMU is still off.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 32 ++
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 434 -------------------
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 435 ++++++++++++++++++++
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +
4 files changed, 469 insertions(+), 434 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
new file mode 100644
index 000000000000..e3b02a9fba57
--- /dev/null
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
@@ -0,0 +1,32 @@
+/** @file
+* File managing the MMU for ARMv7 architecture
+*
+* Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <Uefi.h>
+
+#include <Library/ArmLib.h>
+
+#include <Chipset/ArmV7.h>
+
+UINT32
+ConvertSectionAttributesToPageAttributes (
+ IN UINT32 SectionAttributes,
+ IN BOOLEAN IsLargePage
+ )
+{
+ UINT32 PageAttributes;
+
+ PageAttributes = 0;
+ PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_CACHE_POLICY (SectionAttributes, IsLargePage);
+ PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_AP (SectionAttributes);
+ PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_XN (SectionAttributes, IsLargePage);
+ PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_NG (SectionAttributes);
+ PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_S (SectionAttributes);
+
+ return PageAttributes;
+}
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index a6601258bee0..aca7a37facac 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -31,15 +31,6 @@
#define ID_MMFR0_SHR_IMP_HW_COHERENT 1
#define ID_MMFR0_SHR_IGNORED 0xf
-#define __EFI_MEMORY_RWX 0 // no restrictions
-
-#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | \
- EFI_MEMORY_WC | \
- EFI_MEMORY_WT | \
- EFI_MEMORY_WB | \
- EFI_MEMORY_UCE | \
- EFI_MEMORY_WP)
-
UINTN
EFIAPI
ArmReadIdMmfr0 (
@@ -52,24 +43,6 @@ ArmHasMpExtensions (
VOID
);
-UINT32
-ConvertSectionAttributesToPageAttributes (
- IN UINT32 SectionAttributes,
- IN BOOLEAN IsLargePage
- )
-{
- UINT32 PageAttributes;
-
- PageAttributes = 0;
- PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_CACHE_POLICY (SectionAttributes, IsLargePage);
- PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_AP (SectionAttributes);
- PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_XN (SectionAttributes, IsLargePage);
- PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_NG (SectionAttributes);
- PageAttributes |= TT_DESCRIPTOR_CONVERT_TO_PAGE_S (SectionAttributes);
-
- return PageAttributes;
-}
-
STATIC
BOOLEAN
PreferNonshareableMemory (
@@ -423,410 +396,3 @@ ArmConfigureMmu (
ArmEnableMmu();
return RETURN_SUCCESS;
}
-
-STATIC
-EFI_STATUS
-ConvertSectionToPages (
- IN EFI_PHYSICAL_ADDRESS BaseAddress
- )
-{
- UINT32 FirstLevelIdx;
- UINT32 SectionDescriptor;
- UINT32 PageTableDescriptor;
- UINT32 PageDescriptor;
- UINT32 Index;
-
- volatile ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;
- volatile ARM_PAGE_TABLE_ENTRY *PageTable;
-
- DEBUG ((EFI_D_PAGE, "Converting section at 0x%x to pages\n", (UINTN)BaseAddress));
-
- // Obtain page table base
- FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
-
- // Calculate index into first level translation table for start of modification
- FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
- ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);
-
- // Get section attributes and convert to page attributes
- SectionDescriptor = FirstLevelTable[FirstLevelIdx];
- PageDescriptor = TT_DESCRIPTOR_PAGE_TYPE_PAGE | ConvertSectionAttributesToPageAttributes (SectionDescriptor, FALSE);
-
- // Allocate a page table for the 4KB entries (we use up a full page even though we only need 1KB)
- PageTable = (volatile ARM_PAGE_TABLE_ENTRY *)AllocatePages (1);
- if (PageTable == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- // Write the page table entries out
- for (Index = 0; Index < TRANSLATION_TABLE_PAGE_COUNT; Index++) {
- PageTable[Index] = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(BaseAddress + (Index << 12)) | PageDescriptor;
- }
-
- // Formulate page table entry, Domain=0, NS=0
- PageTableDescriptor = (((UINTN)PageTable) & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK) | TT_DESCRIPTOR_SECTION_TYPE_PAGE_TABLE;
-
- // Write the page table entry out, replacing section entry
- FirstLevelTable[FirstLevelIdx] = PageTableDescriptor;
-
- return EFI_SUCCESS;
-}
-
-STATIC
-EFI_STATUS
-UpdatePageEntries (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes,
- OUT BOOLEAN *FlushTlbs OPTIONAL
- )
-{
- EFI_STATUS Status;
- UINT32 EntryValue;
- UINT32 EntryMask;
- UINT32 FirstLevelIdx;
- UINT32 Offset;
- UINT32 NumPageEntries;
- UINT32 Descriptor;
- UINT32 p;
- UINT32 PageTableIndex;
- UINT32 PageTableEntry;
- UINT32 CurrentPageTableEntry;
- VOID *Mva;
-
- volatile ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;
- volatile ARM_PAGE_TABLE_ENTRY *PageTable;
-
- Status = EFI_SUCCESS;
-
- // EntryMask: bitmask of values to change (1 = change this value, 0 = leave alone)
- // EntryValue: values at bit positions specified by EntryMask
- EntryMask = TT_DESCRIPTOR_PAGE_TYPE_MASK | TT_DESCRIPTOR_PAGE_AP_MASK;
- if (Attributes & EFI_MEMORY_XP) {
- EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE_XN;
- } else {
- EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE;
- }
-
- // Although the PI spec is unclear on this, the GCD guarantees that only
- // one Attribute bit is set at a time, so the order of the conditionals below
- // is irrelevant. If no memory attribute is specified, we preserve whatever
- // memory type is set in the page tables, and update the permission attributes
- // only.
- if (Attributes & EFI_MEMORY_UC) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
- // map to strongly ordered
- EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0
- } else if (Attributes & EFI_MEMORY_WC) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
- // map to normal non-cachable
- EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0
- } else if (Attributes & EFI_MEMORY_WT) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
- // write through with no-allocate
- EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0
- } else if (Attributes & EFI_MEMORY_WB) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
- // write back (with allocate)
- EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1
- } else if (Attributes & CACHE_ATTRIBUTE_MASK) {
- // catch unsupported memory type attributes
- ASSERT (FALSE);
- return EFI_UNSUPPORTED;
- }
-
- if (Attributes & EFI_MEMORY_RO) {
- EntryValue |= TT_DESCRIPTOR_PAGE_AP_RO_RO;
- } else {
- EntryValue |= TT_DESCRIPTOR_PAGE_AP_RW_RW;
- }
-
- // Obtain page table base
- FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
-
- // Calculate number of 4KB page table entries to change
- NumPageEntries = Length / TT_DESCRIPTOR_PAGE_SIZE;
-
- // Iterate for the number of 4KB pages to change
- Offset = 0;
- for(p = 0; p < NumPageEntries; p++) {
- // Calculate index into first level translation table for page table value
-
- FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress + Offset) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
- ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);
-
- // Read the descriptor from the first level page table
- Descriptor = FirstLevelTable[FirstLevelIdx];
-
- // Does this descriptor need to be converted from section entry to 4K pages?
- if (!TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(Descriptor)) {
- Status = ConvertSectionToPages (FirstLevelIdx << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
- if (EFI_ERROR(Status)) {
- // Exit for loop
- break;
- }
-
- // Re-read descriptor
- Descriptor = FirstLevelTable[FirstLevelIdx];
- if (FlushTlbs != NULL) {
- *FlushTlbs = TRUE;
- }
- }
-
- // Obtain page table base address
- PageTable = (ARM_PAGE_TABLE_ENTRY *)TT_DESCRIPTOR_PAGE_BASE_ADDRESS(Descriptor);
-
- // Calculate index into the page table
- PageTableIndex = ((BaseAddress + Offset) & TT_DESCRIPTOR_PAGE_INDEX_MASK) >> TT_DESCRIPTOR_PAGE_BASE_SHIFT;
- ASSERT (PageTableIndex < TRANSLATION_TABLE_PAGE_COUNT);
-
- // Get the entry
- CurrentPageTableEntry = PageTable[PageTableIndex];
-
- // Mask off appropriate fields
- PageTableEntry = CurrentPageTableEntry & ~EntryMask;
-
- // Mask in new attributes and/or permissions
- PageTableEntry |= EntryValue;
-
- if (CurrentPageTableEntry != PageTableEntry) {
- Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
-
- // Only need to update if we are changing the entry
- PageTable[PageTableIndex] = PageTableEntry;
- ArmUpdateTranslationTableEntry ((VOID *)&PageTable[PageTableIndex], Mva);
- }
-
- Status = EFI_SUCCESS;
- Offset += TT_DESCRIPTOR_PAGE_SIZE;
-
- } // End first level translation table loop
-
- return Status;
-}
-
-STATIC
-EFI_STATUS
-UpdateSectionEntries (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes
- )
-{
- EFI_STATUS Status = EFI_SUCCESS;
- UINT32 EntryMask;
- UINT32 EntryValue;
- UINT32 FirstLevelIdx;
- UINT32 NumSections;
- UINT32 i;
- UINT32 CurrentDescriptor;
- UINT32 Descriptor;
- VOID *Mva;
- volatile ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;
-
- // EntryMask: bitmask of values to change (1 = change this value, 0 = leave alone)
- // EntryValue: values at bit positions specified by EntryMask
-
- // Make sure we handle a section range that is unmapped
- EntryMask = TT_DESCRIPTOR_SECTION_TYPE_MASK | TT_DESCRIPTOR_SECTION_XN_MASK |
- TT_DESCRIPTOR_SECTION_AP_MASK;
- EntryValue = TT_DESCRIPTOR_SECTION_TYPE_SECTION;
-
- // Although the PI spec is unclear on this, the GCD guarantees that only
- // one Attribute bit is set at a time, so the order of the conditionals below
- // is irrelevant. If no memory attribute is specified, we preserve whatever
- // memory type is set in the page tables, and update the permission attributes
- // only.
- if (Attributes & EFI_MEMORY_UC) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
- // map to strongly ordered
- EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0
- } else if (Attributes & EFI_MEMORY_WC) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
- // map to normal non-cachable
- EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0
- } else if (Attributes & EFI_MEMORY_WT) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
- // write through with no-allocate
- EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0
- } else if (Attributes & EFI_MEMORY_WB) {
- // modify cacheability attributes
- EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
- // write back (with allocate)
- EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1
- } else if (Attributes & CACHE_ATTRIBUTE_MASK) {
- // catch unsupported memory type attributes
- ASSERT (FALSE);
- return EFI_UNSUPPORTED;
- }
-
- if (Attributes & EFI_MEMORY_RO) {
- EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
- } else {
- EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
- }
-
- if (Attributes & EFI_MEMORY_XP) {
- EntryValue |= TT_DESCRIPTOR_SECTION_XN_MASK;
- }
-
- // obtain page table base
- FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
-
- // calculate index into first level translation table for start of modification
- FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
- ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);
-
- // calculate number of 1MB first level entries this applies to
- NumSections = Length / TT_DESCRIPTOR_SECTION_SIZE;
-
- // iterate through each descriptor
- for(i=0; i<NumSections; i++) {
- CurrentDescriptor = FirstLevelTable[FirstLevelIdx + i];
-
- // has this descriptor already been converted to pages?
- if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(CurrentDescriptor)) {
- // forward this 1MB range to page table function instead
- Status = UpdatePageEntries (
- (FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT,
- TT_DESCRIPTOR_SECTION_SIZE,
- Attributes,
- NULL);
- } else {
- // still a section entry
-
- if (CurrentDescriptor != 0) {
- // mask off appropriate fields
- Descriptor = CurrentDescriptor & ~EntryMask;
- } else {
- Descriptor = ((UINTN)FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT;
- }
-
- // mask in new attributes and/or permissions
- Descriptor |= EntryValue;
-
- if (CurrentDescriptor != Descriptor) {
- Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
-
- // Only need to update if we are changing the descriptor
- FirstLevelTable[FirstLevelIdx + i] = Descriptor;
- ArmUpdateTranslationTableEntry ((VOID *)&FirstLevelTable[FirstLevelIdx + i], Mva);
- }
-
- Status = EFI_SUCCESS;
- }
- }
-
- return Status;
-}
-
-EFI_STATUS
-ArmSetMemoryAttributes (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes
- )
-{
- EFI_STATUS Status;
- UINT64 ChunkLength;
- BOOLEAN FlushTlbs;
-
- if (BaseAddress > (UINT64)MAX_ADDRESS) {
- return EFI_UNSUPPORTED;
- }
-
- Length = MIN (Length, (UINT64)MAX_ADDRESS - BaseAddress + 1);
- if (Length == 0) {
- return EFI_SUCCESS;
- }
-
- FlushTlbs = FALSE;
- while (Length > 0) {
- if ((BaseAddress % TT_DESCRIPTOR_SECTION_SIZE == 0) &&
- Length >= TT_DESCRIPTOR_SECTION_SIZE) {
-
- ChunkLength = Length - Length % TT_DESCRIPTOR_SECTION_SIZE;
-
- DEBUG ((DEBUG_PAGE,
- "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
- BaseAddress, ChunkLength, Attributes));
-
- Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes);
-
- FlushTlbs = TRUE;
- } else {
-
- //
- // Process page by page until the next section boundary, but only if
- // we have more than a section's worth of area to deal with after that.
- //
- ChunkLength = TT_DESCRIPTOR_SECTION_SIZE -
- (BaseAddress % TT_DESCRIPTOR_SECTION_SIZE);
- if (ChunkLength + TT_DESCRIPTOR_SECTION_SIZE > Length) {
- ChunkLength = Length;
- }
-
- DEBUG ((DEBUG_PAGE,
- "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
- BaseAddress, ChunkLength, Attributes));
-
- Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
- &FlushTlbs);
- }
-
- if (EFI_ERROR (Status)) {
- break;
- }
-
- BaseAddress += ChunkLength;
- Length -= ChunkLength;
- }
-
- if (FlushTlbs) {
- ArmInvalidateTlb ();
- }
- return Status;
-}
-
-EFI_STATUS
-ArmSetMemoryRegionNoExec (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length
- )
-{
- return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);
-}
-
-EFI_STATUS
-ArmClearMemoryRegionNoExec (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length
- )
-{
- return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
-}
-
-EFI_STATUS
-ArmSetMemoryRegionReadOnly (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length
- )
-{
- return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);
-}
-
-EFI_STATUS
-ArmClearMemoryRegionReadOnly (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length
- )
-{
- return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
-}
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
new file mode 100644
index 000000000000..3dafe1d964cd
--- /dev/null
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
@@ -0,0 +1,435 @@
+/** @file
+* File managing the MMU for ARMv7 architecture
+*
+* Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <Uefi.h>
+
+#include <Library/ArmLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/CacheMaintenanceLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+#include <Chipset/ArmV7.h>
+
+#define __EFI_MEMORY_RWX 0 // no restrictions
+
+#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | \
+ EFI_MEMORY_WC | \
+ EFI_MEMORY_WT | \
+ EFI_MEMORY_WB | \
+ EFI_MEMORY_UCE | \
+ EFI_MEMORY_WP)
+
+STATIC
+EFI_STATUS
+ConvertSectionToPages (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress
+ )
+{
+ UINT32 FirstLevelIdx;
+ UINT32 SectionDescriptor;
+ UINT32 PageTableDescriptor;
+ UINT32 PageDescriptor;
+ UINT32 Index;
+
+ volatile ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;
+ volatile ARM_PAGE_TABLE_ENTRY *PageTable;
+
+ DEBUG ((EFI_D_PAGE, "Converting section at 0x%x to pages\n", (UINTN)BaseAddress));
+
+ // Obtain page table base
+ FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
+
+ // Calculate index into first level translation table for start of modification
+ FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
+ ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);
+
+ // Get section attributes and convert to page attributes
+ SectionDescriptor = FirstLevelTable[FirstLevelIdx];
+ PageDescriptor = TT_DESCRIPTOR_PAGE_TYPE_PAGE | ConvertSectionAttributesToPageAttributes (SectionDescriptor, FALSE);
+
+ // Allocate a page table for the 4KB entries (we use up a full page even though we only need 1KB)
+ PageTable = (volatile ARM_PAGE_TABLE_ENTRY *)AllocatePages (1);
+ if (PageTable == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ // Write the page table entries out
+ for (Index = 0; Index < TRANSLATION_TABLE_PAGE_COUNT; Index++) {
+ PageTable[Index] = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(BaseAddress + (Index << 12)) | PageDescriptor;
+ }
+
+ // Formulate page table entry, Domain=0, NS=0
+ PageTableDescriptor = (((UINTN)PageTable) & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK) | TT_DESCRIPTOR_SECTION_TYPE_PAGE_TABLE;
+
+ // Write the page table entry out, replacing section entry
+ FirstLevelTable[FirstLevelIdx] = PageTableDescriptor;
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+UpdatePageEntries (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ OUT BOOLEAN *FlushTlbs OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ UINT32 EntryValue;
+ UINT32 EntryMask;
+ UINT32 FirstLevelIdx;
+ UINT32 Offset;
+ UINT32 NumPageEntries;
+ UINT32 Descriptor;
+ UINT32 p;
+ UINT32 PageTableIndex;
+ UINT32 PageTableEntry;
+ UINT32 CurrentPageTableEntry;
+ VOID *Mva;
+
+ volatile ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;
+ volatile ARM_PAGE_TABLE_ENTRY *PageTable;
+
+ Status = EFI_SUCCESS;
+
+ // EntryMask: bitmask of values to change (1 = change this value, 0 = leave alone)
+ // EntryValue: values at bit positions specified by EntryMask
+ EntryMask = TT_DESCRIPTOR_PAGE_TYPE_MASK | TT_DESCRIPTOR_PAGE_AP_MASK;
+ if (Attributes & EFI_MEMORY_XP) {
+ EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE_XN;
+ } else {
+ EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE;
+ }
+
+ // Although the PI spec is unclear on this, the GCD guarantees that only
+ // one Attribute bit is set at a time, so the order of the conditionals below
+ // is irrelevant. If no memory attribute is specified, we preserve whatever
+ // memory type is set in the page tables, and update the permission attributes
+ // only.
+ if (Attributes & EFI_MEMORY_UC) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
+ // map to strongly ordered
+ EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0
+ } else if (Attributes & EFI_MEMORY_WC) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
+ // map to normal non-cachable
+ EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0
+ } else if (Attributes & EFI_MEMORY_WT) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
+ // write through with no-allocate
+ EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0
+ } else if (Attributes & EFI_MEMORY_WB) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK;
+ // write back (with allocate)
+ EntryValue |= TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1
+ } else if (Attributes & CACHE_ATTRIBUTE_MASK) {
+ // catch unsupported memory type attributes
+ ASSERT (FALSE);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (Attributes & EFI_MEMORY_RO) {
+ EntryValue |= TT_DESCRIPTOR_PAGE_AP_RO_RO;
+ } else {
+ EntryValue |= TT_DESCRIPTOR_PAGE_AP_RW_RW;
+ }
+
+ // Obtain page table base
+ FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
+
+ // Calculate number of 4KB page table entries to change
+ NumPageEntries = Length / TT_DESCRIPTOR_PAGE_SIZE;
+
+ // Iterate for the number of 4KB pages to change
+ Offset = 0;
+ for(p = 0; p < NumPageEntries; p++) {
+ // Calculate index into first level translation table for page table value
+
+ FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress + Offset) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
+ ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);
+
+ // Read the descriptor from the first level page table
+ Descriptor = FirstLevelTable[FirstLevelIdx];
+
+ // Does this descriptor need to be converted from section entry to 4K pages?
+ if (!TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(Descriptor)) {
+ Status = ConvertSectionToPages (FirstLevelIdx << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
+ if (EFI_ERROR(Status)) {
+ // Exit for loop
+ break;
+ }
+
+ // Re-read descriptor
+ Descriptor = FirstLevelTable[FirstLevelIdx];
+ if (FlushTlbs != NULL) {
+ *FlushTlbs = TRUE;
+ }
+ }
+
+ // Obtain page table base address
+ PageTable = (ARM_PAGE_TABLE_ENTRY *)TT_DESCRIPTOR_PAGE_BASE_ADDRESS(Descriptor);
+
+ // Calculate index into the page table
+ PageTableIndex = ((BaseAddress + Offset) & TT_DESCRIPTOR_PAGE_INDEX_MASK) >> TT_DESCRIPTOR_PAGE_BASE_SHIFT;
+ ASSERT (PageTableIndex < TRANSLATION_TABLE_PAGE_COUNT);
+
+ // Get the entry
+ CurrentPageTableEntry = PageTable[PageTableIndex];
+
+ // Mask off appropriate fields
+ PageTableEntry = CurrentPageTableEntry & ~EntryMask;
+
+ // Mask in new attributes and/or permissions
+ PageTableEntry |= EntryValue;
+
+ if (CurrentPageTableEntry != PageTableEntry) {
+ Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
+
+ // Only need to update if we are changing the entry
+ PageTable[PageTableIndex] = PageTableEntry;
+ ArmUpdateTranslationTableEntry ((VOID *)&PageTable[PageTableIndex], Mva);
+ }
+
+ Status = EFI_SUCCESS;
+ Offset += TT_DESCRIPTOR_PAGE_SIZE;
+
+ } // End first level translation table loop
+
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+UpdateSectionEntries (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ )
+{
+ EFI_STATUS Status = EFI_SUCCESS;
+ UINT32 EntryMask;
+ UINT32 EntryValue;
+ UINT32 FirstLevelIdx;
+ UINT32 NumSections;
+ UINT32 i;
+ UINT32 CurrentDescriptor;
+ UINT32 Descriptor;
+ VOID *Mva;
+ volatile ARM_FIRST_LEVEL_DESCRIPTOR *FirstLevelTable;
+
+ // EntryMask: bitmask of values to change (1 = change this value, 0 = leave alone)
+ // EntryValue: values at bit positions specified by EntryMask
+
+ // Make sure we handle a section range that is unmapped
+ EntryMask = TT_DESCRIPTOR_SECTION_TYPE_MASK | TT_DESCRIPTOR_SECTION_XN_MASK |
+ TT_DESCRIPTOR_SECTION_AP_MASK;
+ EntryValue = TT_DESCRIPTOR_SECTION_TYPE_SECTION;
+
+ // Although the PI spec is unclear on this, the GCD guarantees that only
+ // one Attribute bit is set at a time, so the order of the conditionals below
+ // is irrelevant. If no memory attribute is specified, we preserve whatever
+ // memory type is set in the page tables, and update the permission attributes
+ // only.
+ if (Attributes & EFI_MEMORY_UC) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
+ // map to strongly ordered
+ EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_STRONGLY_ORDERED; // TEX[2:0] = 0, C=0, B=0
+ } else if (Attributes & EFI_MEMORY_WC) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
+ // map to normal non-cachable
+ EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_NON_CACHEABLE; // TEX [2:0]= 001 = 0x2, B=0, C=0
+ } else if (Attributes & EFI_MEMORY_WT) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
+ // write through with no-allocate
+ EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC; // TEX [2:0] = 0, C=1, B=0
+ } else if (Attributes & EFI_MEMORY_WB) {
+ // modify cacheability attributes
+ EntryMask |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK;
+ // write back (with allocate)
+ EntryValue |= TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1
+ } else if (Attributes & CACHE_ATTRIBUTE_MASK) {
+ // catch unsupported memory type attributes
+ ASSERT (FALSE);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (Attributes & EFI_MEMORY_RO) {
+ EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
+ } else {
+ EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
+ }
+
+ if (Attributes & EFI_MEMORY_XP) {
+ EntryValue |= TT_DESCRIPTOR_SECTION_XN_MASK;
+ }
+
+ // obtain page table base
+ FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
+
+ // calculate index into first level translation table for start of modification
+ FirstLevelIdx = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(BaseAddress) >> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
+ ASSERT (FirstLevelIdx < TRANSLATION_TABLE_SECTION_COUNT);
+
+ // calculate number of 1MB first level entries this applies to
+ NumSections = Length / TT_DESCRIPTOR_SECTION_SIZE;
+
+ // iterate through each descriptor
+ for(i=0; i<NumSections; i++) {
+ CurrentDescriptor = FirstLevelTable[FirstLevelIdx + i];
+
+ // has this descriptor already been converted to pages?
+ if (TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(CurrentDescriptor)) {
+ // forward this 1MB range to page table function instead
+ Status = UpdatePageEntries (
+ (FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT,
+ TT_DESCRIPTOR_SECTION_SIZE,
+ Attributes,
+ NULL);
+ } else {
+ // still a section entry
+
+ if (CurrentDescriptor != 0) {
+ // mask off appropriate fields
+ Descriptor = CurrentDescriptor & ~EntryMask;
+ } else {
+ Descriptor = ((UINTN)FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT;
+ }
+
+ // mask in new attributes and/or permissions
+ Descriptor |= EntryValue;
+
+ if (CurrentDescriptor != Descriptor) {
+ Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
+
+ // Only need to update if we are changing the descriptor
+ FirstLevelTable[FirstLevelIdx + i] = Descriptor;
+ ArmUpdateTranslationTableEntry ((VOID *)&FirstLevelTable[FirstLevelIdx + i], Mva);
+ }
+
+ Status = EFI_SUCCESS;
+ }
+ }
+
+ return Status;
+}
+
+EFI_STATUS
+ArmSetMemoryAttributes (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ )
+{
+ EFI_STATUS Status;
+ UINT64 ChunkLength;
+ BOOLEAN FlushTlbs;
+
+ if (BaseAddress > (UINT64)MAX_ADDRESS) {
+ return EFI_UNSUPPORTED;
+ }
+
+ Length = MIN (Length, (UINT64)MAX_ADDRESS - BaseAddress + 1);
+ if (Length == 0) {
+ return EFI_SUCCESS;
+ }
+
+ FlushTlbs = FALSE;
+ while (Length > 0) {
+ if ((BaseAddress % TT_DESCRIPTOR_SECTION_SIZE == 0) &&
+ Length >= TT_DESCRIPTOR_SECTION_SIZE) {
+
+ ChunkLength = Length - Length % TT_DESCRIPTOR_SECTION_SIZE;
+
+ DEBUG ((DEBUG_PAGE,
+ "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
+ BaseAddress, ChunkLength, Attributes));
+
+ Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes);
+
+ FlushTlbs = TRUE;
+ } else {
+
+ //
+ // Process page by page until the next section boundary, but only if
+ // we have more than a section's worth of area to deal with after that.
+ //
+ ChunkLength = TT_DESCRIPTOR_SECTION_SIZE -
+ (BaseAddress % TT_DESCRIPTOR_SECTION_SIZE);
+ if (ChunkLength + TT_DESCRIPTOR_SECTION_SIZE > Length) {
+ ChunkLength = Length;
+ }
+
+ DEBUG ((DEBUG_PAGE,
+ "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
+ BaseAddress, ChunkLength, Attributes));
+
+ Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
+ &FlushTlbs);
+ }
+
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ BaseAddress += ChunkLength;
+ Length -= ChunkLength;
+ }
+
+ if (FlushTlbs) {
+ ArmInvalidateTlb ();
+ }
+ return Status;
+}
+
+EFI_STATUS
+ArmSetMemoryRegionNoExec (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length
+ )
+{
+ return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);
+}
+
+EFI_STATUS
+ArmClearMemoryRegionNoExec (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length
+ )
+{
+ return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
+}
+
+EFI_STATUS
+ArmSetMemoryRegionReadOnly (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length
+ )
+{
+ return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);
+}
+
+EFI_STATUS
+ArmClearMemoryRegionReadOnly (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length
+ )
+{
+ return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
+}
diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index 3dfe68ba48a6..2a7e7147958c 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -23,7 +23,9 @@ [Sources.AARCH64]
AArch64/ArmMmuLibReplaceEntry.S
[Sources.ARM]
+ Arm/ArmMmuLibConvert.c
Arm/ArmMmuLibCore.c
+ Arm/ArmMmuLibUpdate.c
Arm/ArmMmuLibV7Support.S |GCC
Arm/ArmMmuLibV7Support.asm |RVCT
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/9] ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (2 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 3/9] ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 5/9] ArmPkg/ArmMmuLib AARCH64: " Ard Biesheuvel
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
In the ARM version of ArmMmuLib, we are currently relying on set/way
invalidation to ensure that the caches are in a consistent state with
respect to main memory once we turn the MMU on. Even if set/way
operations were the appropriate method to achieve this, doing an
invalidate-all first and then populating the page table entries creates
a window where page table entries could be loaded speculatively into
the caches before we modify them, and shadow the new values that we
write there.
So let's get rid of the blanket clean/invalidate operations, and instead,
invalidate each section entry before and after it is updated (to address
all the little corner cases that the ARMv7 spec permits), and invalidate
sets of level 2 entries in blocks, using the generic invalidation routine
from CacheMaintenanceLib
On ARMv7, cache maintenance may be required also when the MMU is
enabled, in case the page table walker is not cache coherent. However,
the code being updated here is guaranteed to run only when the MMU is
still off, and so we can disregard the case when the MMU and caches
are on.
Since the MMU and D-cache are already off when we reach this point, we
can drop the MMU and D-cache disables as well. Maintenance of the I-cache
is unnecessary, since we are not modifying any code, and the installed
mapping is guaranteed to be 1:1. This means we can also leave it enabled
while the page table population code is running.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 55 +++++++++++++++-----
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index aca7a37facac..7c7cad2c3d9d 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -178,11 +178,25 @@ PopulateLevel2PageTable (
ASSERT (FirstPageOffset + Pages <= TRANSLATION_TABLE_PAGE_COUNT);
+ //
+ // Invalidate once to prevent page table updates to hit in the
+ // caches inadvertently.
+ //
+ InvalidateDataCacheRange ((UINT32 *)TranslationTable + FirstPageOffset,
+ RemainLength / TT_DESCRIPTOR_PAGE_SIZE * sizeof (*PageEntry));
+
for (Index = 0; Index < Pages; Index++) {
*PageEntry++ = TT_DESCRIPTOR_PAGE_BASE_ADDRESS(PhysicalBase) | PageAttributes;
PhysicalBase += TT_DESCRIPTOR_PAGE_SIZE;
}
+ //
+ // Invalidate again to ensure that any line fetches that may have occurred
+ // [speculatively] since the previous invalidate are evicted again.
+ //
+ ArmDataMemoryBarrier ();
+ InvalidateDataCacheRange ((UINT32 *)TranslationTable + FirstPageOffset,
+ RemainLength / TT_DESCRIPTOR_PAGE_SIZE * sizeof (*PageEntry));
}
STATIC
@@ -253,11 +267,28 @@ FillTranslationTable (
SectionEntry = TRANSLATION_TABLE_ENTRY_FOR_VIRTUAL_ADDRESS(TranslationTable, MemoryRegion->VirtualBase);
while (RemainLength != 0) {
+ //
+ // Ensure that the assignment of the page table entry will not hit
+ // in the cache. Whether this could occur is IMPLEMENTATION DEFINED
+ // and thus permitted by the ARMv7 architecture.
+ //
+ ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry);
+ ArmDataSynchronizationBarrier ();
+
if (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE == 0 &&
RemainLength >= TT_DESCRIPTOR_SECTION_SIZE) {
// Case: Physical address aligned on the Section Size (1MB) && the length
// is greater than the Section Size
- *SectionEntry++ = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(PhysicalBase) | Attributes;
+ *SectionEntry = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(PhysicalBase) | Attributes;
+
+ //
+ // Issue a DMB to ensure that the page table entry update made it to
+ // memory before we issue the invalidate, otherwise, a subsequent
+ // speculative fetch could observe the old value.
+ //
+ ArmDataMemoryBarrier ();
+ ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry++);
+
PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE;
RemainLength -= TT_DESCRIPTOR_SECTION_SIZE;
} else {
@@ -267,9 +298,17 @@ FillTranslationTable (
// Case: Physical address aligned on the Section Size (1MB) && the length
// does not fill a section
// Case: Physical address NOT aligned on the Section Size (1MB)
- PopulateLevel2PageTable (SectionEntry++, PhysicalBase, PageMapLength,
+ PopulateLevel2PageTable (SectionEntry, PhysicalBase, PageMapLength,
MemoryRegion->Attributes);
+ //
+ // Issue a DMB to ensure that the page table entry update made it to
+ // memory before we issue the invalidate, otherwise, a subsequent
+ // speculative fetch could observe the old value.
+ //
+ ArmDataMemoryBarrier ();
+ ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry++);
+
// If it is the last entry
if (RemainLength < TT_DESCRIPTOR_SECTION_SIZE) {
break;
@@ -349,18 +388,6 @@ ArmConfigureMmu (
}
}
- ArmCleanInvalidateDataCache ();
- ArmInvalidateInstructionCache ();
-
- ArmDisableDataCache ();
- ArmDisableInstructionCache();
- // TLBs are also invalidated when calling ArmDisableMmu()
- ArmDisableMmu ();
-
- // Make sure nothing sneaked into the cache
- ArmCleanInvalidateDataCache ();
- ArmInvalidateInstructionCache ();
-
ArmSetTTBR0 ((VOID *)(UINTN)(((UINTN)TranslationTable & ~TRANSLATION_TABLE_SECTION_ALIGNMENT_MASK) | (TTBRAttributes & 0x7F)));
//
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/9] ArmPkg/ArmMmuLib AARCH64: cache-invalidate initial page table entries
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (3 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 4/9] ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 6/9] ArmPkg/ArmLib: move set/way helper functions into private header Ard Biesheuvel
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
In the AARCH64 version of ArmMmuLib, we are currently relying on
set/way invalidation to ensure that the caches are in a consistent
state with respect to main memory once we turn the MMU on. Even if
set/way operations were the appropriate method to achieve this, doing
an invalidate-all first and then populating the page table entries
creates a window where page table entries could be loaded speculatively
into the caches before we modify them, and shadow the new values that
we write there.
So let's get rid of the blanket clean/invalidate operations, and
instead, update ArmUpdateTranslationTableEntry () to invalidate each
page table entry *after* it is written if the MMU is still disabled
at this point.
On ARMv8, it is guaranteed that memory accesses done by the page table
walker are cache coherent, and so we can ignore the case where the
MMU is on.
Since the MMU and D-cache are already off when we reach this point, we
can drop the MMU and D-cache disables as well. Maintenance of the I-cache
is unnecessary, since we are not modifying any code, and the installed
mapping is guaranteed to be 1:1. This means we can also leave it enabled
while the page table population code is running.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 9 ++++++++-
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 9 ---------
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index 1adf960377a2..f744cd6738b9 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -13,6 +13,8 @@
.set DAIF_RD_FIQ_BIT, (1 << 6)
.set DAIF_RD_IRQ_BIT, (1 << 7)
+.set SCTLR_ELx_M_BIT_POS, (0)
+
ASM_FUNC(ArmReadMidr)
mrs x0, midr_el1 // Read from Main ID Register (MIDR)
ret
@@ -122,11 +124,16 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)
lsr x1, x1, #12
EL1_OR_EL2_OR_EL3(x0)
1: tlbi vaae1, x1 // TLB Invalidate VA , EL1
+ mrs x2, sctlr_el1
b 4f
2: tlbi vae2, x1 // TLB Invalidate VA , EL2
+ mrs x2, sctlr_el2
b 4f
3: tlbi vae3, x1 // TLB Invalidate VA , EL3
-4: dsb nsh
+ mrs x2, sctlr_el3
+4: tbnz x2, SCTLR_ELx_M_BIT_POS, 5f
+ dc ivac, x0 // invalidate in Dcache if MMU is still off
+5: dsb nsh
isb
ret
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index e8f5c69e3136..204e33c75f95 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -699,15 +699,6 @@ ArmConfigureMmu (
ZeroMem (TranslationTable, RootTableEntryCount * sizeof(UINT64));
- // Disable MMU and caches. ArmDisableMmu() also invalidates the TLBs
- ArmDisableMmu ();
- ArmDisableDataCache ();
- ArmDisableInstructionCache ();
-
- // Make sure nothing sneaked into the cache
- ArmCleanInvalidateDataCache ();
- ArmInvalidateInstructionCache ();
-
TranslationTableAttribute = TT_ATTR_INDX_INVALID;
while (MemoryTable->Length != 0) {
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 6/9] ArmPkg/ArmLib: move set/way helper functions into private header
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (4 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 5/9] ArmPkg/ArmMmuLib AARCH64: " Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 7/9] ArmPkg/ArmLib: clean up library includes Ard Biesheuvel
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
The clean/invalidate helper functions that operate on a single cache
line identified by set, way and level in a special, architected format
are only used by the implementations of the clean/invalidate routines
that operate on the entire cache hierarchy, as exposed by ArmLib.
The latter routines will be deprecated soon, so move the helpers out
of ArmLib.h and into a private header so they are safe from abuse.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Include/Library/ArmLib.h | 18 ------------------
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 18 ++++++++++++++++++
ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h | 18 ++++++++++++++++++
3 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index e76a46d5f4ce..5a27b7c2fc27 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -211,24 +211,6 @@ ArmCleanInvalidateDataCacheEntryByMVA (
IN UINTN Address
);
-VOID
-EFIAPI
-ArmInvalidateDataCacheEntryBySetWay (
- IN UINTN SetWayFormat
- );
-
-VOID
-EFIAPI
-ArmCleanDataCacheEntryBySetWay (
- IN UINTN SetWayFormat
- );
-
-VOID
-EFIAPI
-ArmCleanInvalidateDataCacheEntryBySetWay (
- IN UINTN SetWayFormat
- );
-
VOID
EFIAPI
ArmEnableDataCache (
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h
index ab9bcf553c4d..b2c8a8ea0b84 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h
@@ -17,5 +17,23 @@ AArch64AllDataCachesOperation (
IN AARCH64_CACHE_OPERATION DataCacheOperation
);
+VOID
+EFIAPI
+ArmInvalidateDataCacheEntryBySetWay (
+ IN UINTN SetWayFormat
+ );
+
+VOID
+EFIAPI
+ArmCleanDataCacheEntryBySetWay (
+ IN UINTN SetWayFormat
+ );
+
+VOID
+EFIAPI
+ArmCleanInvalidateDataCacheEntryBySetWay (
+ IN UINTN SetWayFormat
+ );
+
#endif // __AARCH64_LIB_H__
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h
index c52fb9a1b484..93183e67230e 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h
@@ -30,5 +30,23 @@ ArmV7AllDataCachesOperation (
IN ARM_V7_CACHE_OPERATION DataCacheOperation
);
+VOID
+EFIAPI
+ArmInvalidateDataCacheEntryBySetWay (
+ IN UINTN SetWayFormat
+ );
+
+VOID
+EFIAPI
+ArmCleanDataCacheEntryBySetWay (
+ IN UINTN SetWayFormat
+ );
+
+VOID
+EFIAPI
+ArmCleanInvalidateDataCacheEntryBySetWay (
+ IN UINTN SetWayFormat
+ );
+
#endif // __ARM_V7_LIB_H__
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 7/9] ArmPkg/ArmLib: clean up library includes
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (5 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 6/9] ArmPkg/ArmLib: move set/way helper functions into private header Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 8/9] ArmPkg/ArmLib: remove bogus protocol declaration Ard Biesheuvel
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
Suspiciously, ArmLib's INF does not contain a [LibraryClasses]
section at all, but it turns out that all the library includes
it contains (except for ArmLib.h itself) are actually bogus so
let's just drop all of them. While at it, replace <Uefi.h> with
the more accurate <Base.h> for a BASE type module, and put the
includes in a consistent order.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 9 +++++----
ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 10 ++++++----
ArmPkg/Library/ArmLib/ArmLib.c | 2 --
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
index 0ed8dae9a4b0..924bf48020e0 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
@@ -7,11 +7,12 @@
**/
-#include <Uefi.h>
-#include <Chipset/AArch64.h>
+#include <Base.h>
+
#include <Library/ArmLib.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
+
+#include <Chipset/AArch64.h>
+
#include "AArch64Lib.h"
#include "ArmLibPrivate.h"
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
index 38516d4f1b87..5d93aa6e0b8c 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
@@ -6,11 +6,13 @@
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
-#include <Uefi.h>
-#include <Chipset/ArmV7.h>
+
+#include <Base.h>
+
#include <Library/ArmLib.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
+
+#include <Chipset/ArmV7.h>
+
#include "ArmV7Lib.h"
#include "ArmLibPrivate.h"
diff --git a/ArmPkg/Library/ArmLib/ArmLib.c b/ArmPkg/Library/ArmLib/ArmLib.c
index c682c3ab6339..3905d02c5e7e 100644
--- a/ArmPkg/Library/ArmLib/ArmLib.c
+++ b/ArmPkg/Library/ArmLib/ArmLib.c
@@ -10,8 +10,6 @@
#include <Base.h>
#include <Library/ArmLib.h>
-#include <Library/DebugLib.h>
-#include <Library/PcdLib.h>
#include "ArmLibPrivate.h"
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 8/9] ArmPkg/ArmLib: remove bogus protocol declaration
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (6 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 7/9] ArmPkg/ArmLib: clean up library includes Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-04 18:12 ` [PATCH v2 9/9] ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on Ard Biesheuvel
2020-03-05 16:29 ` [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Leif Lindholm
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
ArmLib is a BASE type library, which should not depend or
even be aware on DXE type protocols. So drop the reference
to gEfiCpuArchProtocolGuid.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmLib/ArmBaseLib.inf | 3 ---
1 file changed, 3 deletions(-)
diff --git a/ArmPkg/Library/ArmLib/ArmBaseLib.inf b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
index 5e70990872f2..106a09f821e1 100644
--- a/ArmPkg/Library/ArmLib/ArmBaseLib.inf
+++ b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
@@ -48,8 +48,5 @@ [Packages]
ArmPkg/ArmPkg.dec
MdePkg/MdePkg.dec
-[Protocols]
- gEfiCpuArchProtocolGuid
-
[FeaturePcd.ARM]
gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 9/9] ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (7 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 8/9] ArmPkg/ArmLib: remove bogus protocol declaration Ard Biesheuvel
@ 2020-03-04 18:12 ` Ard Biesheuvel
2020-03-05 16:29 ` [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Leif Lindholm
9 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 18:12 UTC (permalink / raw)
To: devel; +Cc: leif, Ard Biesheuvel
On ARMv7 and up, doing cache maintenance by set/way is only
permitted in the context of on/offlining a core, and any other
uses should be avoided. Add ASSERT()s in the right place to
ensure that any uses with the MMU enabled are caught in DEBUG
builds.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 7 +++++++
ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 7 +++++++
ArmPkg/Library/ArmLib/ArmBaseLib.inf | 3 +++
3 files changed, 17 insertions(+)
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
index 924bf48020e0..3fbd591192e2 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
@@ -10,6 +10,7 @@
#include <Base.h>
#include <Library/ArmLib.h>
+#include <Library/DebugLib.h>
#include <Chipset/AArch64.h>
@@ -41,6 +42,8 @@ ArmInvalidateDataCache (
VOID
)
{
+ ASSERT (!ArmMmuEnabled ());
+
ArmDataSynchronizationBarrier ();
AArch64DataCacheOperation (ArmInvalidateDataCacheEntryBySetWay);
}
@@ -51,6 +54,8 @@ ArmCleanInvalidateDataCache (
VOID
)
{
+ ASSERT (!ArmMmuEnabled ());
+
ArmDataSynchronizationBarrier ();
AArch64DataCacheOperation (ArmCleanInvalidateDataCacheEntryBySetWay);
}
@@ -61,6 +66,8 @@ ArmCleanDataCache (
VOID
)
{
+ ASSERT (!ArmMmuEnabled ());
+
ArmDataSynchronizationBarrier ();
AArch64DataCacheOperation (ArmCleanDataCacheEntryBySetWay);
}
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
index 5d93aa6e0b8c..2c4a23e1a1b2 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
@@ -10,6 +10,7 @@
#include <Base.h>
#include <Library/ArmLib.h>
+#include <Library/DebugLib.h>
#include <Chipset/ArmV7.h>
@@ -41,6 +42,8 @@ ArmInvalidateDataCache (
VOID
)
{
+ ASSERT (!ArmMmuEnabled ());
+
ArmDataSynchronizationBarrier ();
ArmV7DataCacheOperation (ArmInvalidateDataCacheEntryBySetWay);
}
@@ -51,6 +54,8 @@ ArmCleanInvalidateDataCache (
VOID
)
{
+ ASSERT (!ArmMmuEnabled ());
+
ArmDataSynchronizationBarrier ();
ArmV7DataCacheOperation (ArmCleanInvalidateDataCacheEntryBySetWay);
}
@@ -61,6 +66,8 @@ ArmCleanDataCache (
VOID
)
{
+ ASSERT (!ArmMmuEnabled ());
+
ArmDataSynchronizationBarrier ();
ArmV7DataCacheOperation (ArmCleanDataCacheEntryBySetWay);
}
diff --git a/ArmPkg/Library/ArmLib/ArmBaseLib.inf b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
index 106a09f821e1..f61c71b673d1 100644
--- a/ArmPkg/Library/ArmLib/ArmBaseLib.inf
+++ b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
@@ -44,6 +44,9 @@ [Sources.AARCH64]
AArch64/AArch64Support.S
AArch64/AArch64ArchTimerSupport.S
+[LibraryClasses]
+ DebugLib
+
[Packages]
ArmPkg/ArmPkg.dec
MdePkg/MdePkg.dec
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops
2020-03-04 18:12 [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Ard Biesheuvel
` (8 preceding siblings ...)
2020-03-04 18:12 ` [PATCH v2 9/9] ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on Ard Biesheuvel
@ 2020-03-05 16:29 ` Leif Lindholm
2020-03-05 21:40 ` Ard Biesheuvel
9 siblings, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2020-03-05 16:29 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel
On Wed, Mar 04, 2020 at 19:12:37 +0100, Ard Biesheuvel wrote:
> This is a combination of v1 'ArmPkg: eradicate and deprecate by set/way cache
> ops' and v1 'ArmPkg/ArmLib: ASSERT() on misuse of set/way ops'
>
> As it turns out, there were still some instances of set/way ops left in
> the core code, in ArmMmuLib to be precise.
>
> This series fixes ArmMmuLib to perform the appropriate cache invalidation
> when populating page tables with the MMU and caches off, allowing us to
> get rid of the cache clean/disable/enable sequences which are incorrect
> and pointless at the same time.
>
> I have incorporated some ArmLib changes that I posted separately before,
> with the end result being that all uses pf set/way ops are gone from the
> EDK2 core code, and the routines themselves will now ASSERT() when used
> for anything other than managing the caches while the MMU is still off.
> (Note that BeagleBoard in edk2-platforms still relies on this)
>
> Changes since v1[s]:
> - don't deprecate the set/way ops but make the usable before MMU is enabled
> only
> - use a more elaborate sequence for invalidating the page table entries on
> 32-bit ARM, to ensure we are compliant with the nooks and crannies of
> version 7 of the architecture.
> - incorporate patch #1, which was still pending on the list, and is related
> (its PrePeiCore sibling was already reviewed and merged)
>
> Ard Biesheuvel (9):
> ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones
> ArmPkg/ArmMmuLib ARM: remove dummy constructor
> ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code
> ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries
> ArmPkg/ArmMmuLib AARCH64: cache-invalidate initial page table entries
> ArmPkg/ArmLib: move set/way helper functions into private header
> ArmPkg/ArmLib: clean up library includes
> ArmPkg/ArmLib: remove bogus protocol declaration
> ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on
Apart from the minor nit pointed out (which you can fix before
committing) - for the series:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Thanks!
> ArmPkg/Include/Library/ArmLib.h | 18 -
> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 16 +-
> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 18 +
> ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 9 +-
> ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c | 17 +-
> ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h | 18 +
> ArmPkg/Library/ArmLib/ArmBaseLib.inf | 6 +-
> ArmPkg/Library/ArmLib/ArmLib.c | 2 -
> .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 9 -
> .../Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 32 ++
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 498 ++----------------
> .../Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 435 +++++++++++++++
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 +
> ArmPlatformPkg/PrePi/PeiMPCore.inf | 1 +
> ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 +
> ArmPlatformPkg/PrePi/PrePi.c | 8 +-
> 16 files changed, 591 insertions(+), 501 deletions(-)
> create mode 100644 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
> create mode 100644 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops
2020-03-05 16:29 ` [PATCH v2 0/9] ArmPkg: eradicate and deprecate by set/way cache ops Leif Lindholm
@ 2020-03-05 21:40 ` Ard Biesheuvel
0 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-03-05 21:40 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io
On Thu, 5 Mar 2020 at 17:29, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Wed, Mar 04, 2020 at 19:12:37 +0100, Ard Biesheuvel wrote:
> > This is a combination of v1 'ArmPkg: eradicate and deprecate by set/way cache
> > ops' and v1 'ArmPkg/ArmLib: ASSERT() on misuse of set/way ops'
> >
> > As it turns out, there were still some instances of set/way ops left in
> > the core code, in ArmMmuLib to be precise.
> >
> > This series fixes ArmMmuLib to perform the appropriate cache invalidation
> > when populating page tables with the MMU and caches off, allowing us to
> > get rid of the cache clean/disable/enable sequences which are incorrect
> > and pointless at the same time.
> >
> > I have incorporated some ArmLib changes that I posted separately before,
> > with the end result being that all uses pf set/way ops are gone from the
> > EDK2 core code, and the routines themselves will now ASSERT() when used
> > for anything other than managing the caches while the MMU is still off.
> > (Note that BeagleBoard in edk2-platforms still relies on this)
> >
> > Changes since v1[s]:
> > - don't deprecate the set/way ops but make the usable before MMU is enabled
> > only
> > - use a more elaborate sequence for invalidating the page table entries on
> > 32-bit ARM, to ensure we are compliant with the nooks and crannies of
> > version 7 of the architecture.
> > - incorporate patch #1, which was still pending on the list, and is related
> > (its PrePeiCore sibling was already reviewed and merged)
> >
> > Ard Biesheuvel (9):
> > ArmPlatformPkg/PrePi: replace set/way cache ops with by-VA ones
> > ArmPkg/ArmMmuLib ARM: remove dummy constructor
> > ArmPkg/ArmMmuLib ARM: split ArmMmuLibCore.c into core and update code
> > ArmPkg/ArmMmuLib ARM: cache-invalidate initial page table entries
> > ArmPkg/ArmMmuLib AARCH64: cache-invalidate initial page table entries
> > ArmPkg/ArmLib: move set/way helper functions into private header
> > ArmPkg/ArmLib: clean up library includes
> > ArmPkg/ArmLib: remove bogus protocol declaration
> > ArmPkg/ArmLib: ASSERT on set/way cache ops being used with MMU on
>
> Apart from the minor nit pointed out (which you can fix before
> committing) - for the series:
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>
Thanks. These are merged now, along with the followup series, and the
only vaguely related cleanups for ARM.
I did spot one other issue on AArch64 though, so I'll send out another
followup patch to address that.
^ permalink raw reply [flat|nested] 13+ messages in thread