* [PATCH 1/5] ArmPkg/ArmMmuLib AARCH64: use correct return type for exported functions
2017-03-01 16:31 [PATCH 0/5] ArmPkg, ArmVirtPkg ARM: enable non-executable stack Ard Biesheuvel
@ 2017-03-01 16:31 ` Ard Biesheuvel
2017-03-06 14:57 ` Leif Lindholm
2017-03-01 16:31 ` [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-01 16:31 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
The routines ArmConfigureMmu() and SetMemoryAttributes() [*] are
declared as returning EFI_STATUS in the respective header files,
so align the definitions with that.
* SetMemoryAttributes() is declared in the wrong header (and defined in
ArmMmuLib for AARCH64 and in CpuDxe for ARM)
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 ++++++++++----------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 2f8f99d44a31..df170d20a2c2 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -329,7 +329,7 @@ GetBlockEntryListFromAddress (
}
STATIC
-RETURN_STATUS
+EFI_STATUS
UpdateRegionMapping (
IN UINT64 *RootTable,
IN UINT64 RegionStart,
@@ -347,7 +347,7 @@ UpdateRegionMapping (
// Ensure the Length is aligned on 4KB boundary
if ((RegionLength == 0) || ((RegionLength & (SIZE_4KB - 1)) != 0)) {
ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
- return RETURN_INVALID_PARAMETER;
+ return EFI_INVALID_PARAMETER;
}
do {
@@ -357,7 +357,7 @@ UpdateRegionMapping (
BlockEntry = GetBlockEntryListFromAddress (RootTable, RegionStart, &TableLevel, &BlockEntrySize, &LastBlockEntry);
if (BlockEntry == NULL) {
// GetBlockEntryListFromAddress() return NULL when it fails to allocate new pages from the Translation Tables
- return RETURN_OUT_OF_RESOURCES;
+ return EFI_OUT_OF_RESOURCES;
}
if (TableLevel != 3) {
@@ -385,11 +385,11 @@ UpdateRegionMapping (
} while ((RegionLength >= BlockEntrySize) && (BlockEntry <= LastBlockEntry));
} while (RegionLength != 0);
- return RETURN_SUCCESS;
+ return EFI_SUCCESS;
}
STATIC
-RETURN_STATUS
+EFI_STATUS
FillTranslationTable (
IN UINT64 *RootTable,
IN ARM_MEMORY_REGION_DESCRIPTOR *MemoryRegion
@@ -446,7 +446,7 @@ GcdAttributeToPageAttribute (
return PageAttributes | TT_AF;
}
-RETURN_STATUS
+EFI_STATUS
SetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
@@ -454,7 +454,7 @@ SetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS VirtualMask
)
{
- RETURN_STATUS Status;
+ EFI_STATUS Status;
UINT64 *TranslationTable;
UINT64 PageAttributes;
UINT64 PageAttributeMask;
@@ -480,18 +480,18 @@ SetMemoryAttributes (
Length,
PageAttributes,
PageAttributeMask);
- if (RETURN_ERROR (Status)) {
+ if (EFI_ERROR (Status)) {
return Status;
}
// Invalidate all TLB entries so changes are synced
ArmInvalidateTlb ();
- return RETURN_SUCCESS;
+ return EFI_SUCCESS;
}
STATIC
-RETURN_STATUS
+EFI_STATUS
SetMemoryRegionAttribute (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
@@ -499,23 +499,23 @@ SetMemoryRegionAttribute (
IN UINT64 BlockEntryMask
)
{
- RETURN_STATUS Status;
+ EFI_STATUS Status;
UINT64 *RootTable;
RootTable = ArmGetTTBR0BaseAddress ();
Status = UpdateRegionMapping (RootTable, BaseAddress, Length, Attributes, BlockEntryMask);
- if (RETURN_ERROR (Status)) {
+ if (EFI_ERROR (Status)) {
return Status;
}
// Invalidate all TLB entries so changes are synced
ArmInvalidateTlb ();
- return RETURN_SUCCESS;
+ return EFI_SUCCESS;
}
-RETURN_STATUS
+EFI_STATUS
ArmSetMemoryRegionNoExec (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
@@ -536,7 +536,7 @@ ArmSetMemoryRegionNoExec (
~TT_ADDRESS_MASK_BLOCK_ENTRY);
}
-RETURN_STATUS
+EFI_STATUS
ArmClearMemoryRegionNoExec (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
@@ -554,7 +554,7 @@ ArmClearMemoryRegionNoExec (
Mask);
}
-RETURN_STATUS
+EFI_STATUS
ArmSetMemoryRegionReadOnly (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
@@ -567,7 +567,7 @@ ArmSetMemoryRegionReadOnly (
~TT_ADDRESS_MASK_BLOCK_ENTRY);
}
-RETURN_STATUS
+EFI_STATUS
ArmClearMemoryRegionReadOnly (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
@@ -580,7 +580,7 @@ ArmClearMemoryRegionReadOnly (
~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK));
}
-RETURN_STATUS
+EFI_STATUS
EFIAPI
ArmConfigureMmu (
IN ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable,
@@ -594,11 +594,11 @@ ArmConfigureMmu (
UINTN T0SZ;
UINTN RootTableEntryCount;
UINT64 TCR;
- RETURN_STATUS Status;
+ EFI_STATUS Status;
if(MemoryTable == NULL) {
ASSERT (MemoryTable != NULL);
- return RETURN_INVALID_PARAMETER;
+ return EFI_INVALID_PARAMETER;
}
// Cover the entire GCD memory space
@@ -632,7 +632,7 @@ ArmConfigureMmu (
} else {
DEBUG ((EFI_D_ERROR, "ArmConfigureMmu: The MaxAddress 0x%lX is not supported by this MMU configuration.\n", MaxAddress));
ASSERT (0); // Bigger than 48-bit memory space are not supported
- return RETURN_UNSUPPORTED;
+ return EFI_UNSUPPORTED;
}
} else if (ArmReadCurrentEL () == AARCH64_EL1) {
// Due to Cortex-A57 erratum #822227 we must set TG1[1] == 1, regardless of EPD1.
@@ -654,11 +654,11 @@ ArmConfigureMmu (
} else {
DEBUG ((EFI_D_ERROR, "ArmConfigureMmu: The MaxAddress 0x%lX is not supported by this MMU configuration.\n", MaxAddress));
ASSERT (0); // Bigger than 48-bit memory space are not supported
- return RETURN_UNSUPPORTED;
+ return EFI_UNSUPPORTED;
}
} else {
ASSERT (0); // UEFI is only expected to run at EL2 and EL1, not EL3.
- return RETURN_UNSUPPORTED;
+ return EFI_UNSUPPORTED;
}
//
@@ -680,7 +680,7 @@ ArmConfigureMmu (
// Allocate pages for translation table
TranslationTable = AllocatePages (1);
if (TranslationTable == NULL) {
- return RETURN_OUT_OF_RESOURCES;
+ return EFI_OUT_OF_RESOURCES;
}
// We set TTBR0 just after allocating the table to retrieve its location from the subsequent
// functions without needing to pass this value across the functions. The MMU is only enabled
@@ -719,7 +719,7 @@ ArmConfigureMmu (
DEBUG_CODE_END ();
Status = FillTranslationTable (TranslationTable, MemoryTable);
- if (RETURN_ERROR (Status)) {
+ if (EFI_ERROR (Status)) {
goto FREE_TRANSLATION_TABLE;
}
MemoryTable++;
@@ -739,7 +739,7 @@ ArmConfigureMmu (
ArmEnableDataCache ();
ArmEnableMmu ();
- return RETURN_SUCCESS;
+ return EFI_SUCCESS;
FREE_TRANSLATION_TABLE:
FreePages (TranslationTable, 1);
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/5] ArmPkg/ArmMmuLib AARCH64: use correct return type for exported functions
2017-03-01 16:31 ` [PATCH 1/5] ArmPkg/ArmMmuLib AARCH64: use correct return type for exported functions Ard Biesheuvel
@ 2017-03-06 14:57 ` Leif Lindholm
0 siblings, 0 replies; 14+ messages in thread
From: Leif Lindholm @ 2017-03-06 14:57 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek
On Wed, Mar 01, 2017 at 04:31:39PM +0000, Ard Biesheuvel wrote:
> The routines ArmConfigureMmu() and SetMemoryAttributes() [*] are
> declared as returning EFI_STATUS in the respective header files,
> so align the definitions with that.
>
Ouch. Well, in general, this is just not very EDK2:ish.
> * SetMemoryAttributes() is declared in the wrong header (and defined in
> ArmMmuLib for AARCH64 and in CpuDxe for ARM)
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
No need to wait for rest of series (or prereqs for), this one stands alone.
> ---
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 52 ++++++++++----------
> 1 file changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 2f8f99d44a31..df170d20a2c2 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -329,7 +329,7 @@ GetBlockEntryListFromAddress (
> }
>
> STATIC
> -RETURN_STATUS
> +EFI_STATUS
> UpdateRegionMapping (
> IN UINT64 *RootTable,
> IN UINT64 RegionStart,
> @@ -347,7 +347,7 @@ UpdateRegionMapping (
> // Ensure the Length is aligned on 4KB boundary
> if ((RegionLength == 0) || ((RegionLength & (SIZE_4KB - 1)) != 0)) {
> ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> - return RETURN_INVALID_PARAMETER;
> + return EFI_INVALID_PARAMETER;
> }
>
> do {
> @@ -357,7 +357,7 @@ UpdateRegionMapping (
> BlockEntry = GetBlockEntryListFromAddress (RootTable, RegionStart, &TableLevel, &BlockEntrySize, &LastBlockEntry);
> if (BlockEntry == NULL) {
> // GetBlockEntryListFromAddress() return NULL when it fails to allocate new pages from the Translation Tables
> - return RETURN_OUT_OF_RESOURCES;
> + return EFI_OUT_OF_RESOURCES;
> }
>
> if (TableLevel != 3) {
> @@ -385,11 +385,11 @@ UpdateRegionMapping (
> } while ((RegionLength >= BlockEntrySize) && (BlockEntry <= LastBlockEntry));
> } while (RegionLength != 0);
>
> - return RETURN_SUCCESS;
> + return EFI_SUCCESS;
> }
>
> STATIC
> -RETURN_STATUS
> +EFI_STATUS
> FillTranslationTable (
> IN UINT64 *RootTable,
> IN ARM_MEMORY_REGION_DESCRIPTOR *MemoryRegion
> @@ -446,7 +446,7 @@ GcdAttributeToPageAttribute (
> return PageAttributes | TT_AF;
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> SetMemoryAttributes (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> @@ -454,7 +454,7 @@ SetMemoryAttributes (
> IN EFI_PHYSICAL_ADDRESS VirtualMask
> )
> {
> - RETURN_STATUS Status;
> + EFI_STATUS Status;
> UINT64 *TranslationTable;
> UINT64 PageAttributes;
> UINT64 PageAttributeMask;
> @@ -480,18 +480,18 @@ SetMemoryAttributes (
> Length,
> PageAttributes,
> PageAttributeMask);
> - if (RETURN_ERROR (Status)) {
> + if (EFI_ERROR (Status)) {
> return Status;
> }
>
> // Invalidate all TLB entries so changes are synced
> ArmInvalidateTlb ();
>
> - return RETURN_SUCCESS;
> + return EFI_SUCCESS;
> }
>
> STATIC
> -RETURN_STATUS
> +EFI_STATUS
> SetMemoryRegionAttribute (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> @@ -499,23 +499,23 @@ SetMemoryRegionAttribute (
> IN UINT64 BlockEntryMask
> )
> {
> - RETURN_STATUS Status;
> + EFI_STATUS Status;
> UINT64 *RootTable;
>
> RootTable = ArmGetTTBR0BaseAddress ();
>
> Status = UpdateRegionMapping (RootTable, BaseAddress, Length, Attributes, BlockEntryMask);
> - if (RETURN_ERROR (Status)) {
> + if (EFI_ERROR (Status)) {
> return Status;
> }
>
> // Invalidate all TLB entries so changes are synced
> ArmInvalidateTlb ();
>
> - return RETURN_SUCCESS;
> + return EFI_SUCCESS;
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmSetMemoryRegionNoExec (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> @@ -536,7 +536,7 @@ ArmSetMemoryRegionNoExec (
> ~TT_ADDRESS_MASK_BLOCK_ENTRY);
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmClearMemoryRegionNoExec (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> @@ -554,7 +554,7 @@ ArmClearMemoryRegionNoExec (
> Mask);
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmSetMemoryRegionReadOnly (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> @@ -567,7 +567,7 @@ ArmSetMemoryRegionReadOnly (
> ~TT_ADDRESS_MASK_BLOCK_ENTRY);
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmClearMemoryRegionReadOnly (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> @@ -580,7 +580,7 @@ ArmClearMemoryRegionReadOnly (
> ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK));
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> EFIAPI
> ArmConfigureMmu (
> IN ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable,
> @@ -594,11 +594,11 @@ ArmConfigureMmu (
> UINTN T0SZ;
> UINTN RootTableEntryCount;
> UINT64 TCR;
> - RETURN_STATUS Status;
> + EFI_STATUS Status;
>
> if(MemoryTable == NULL) {
> ASSERT (MemoryTable != NULL);
> - return RETURN_INVALID_PARAMETER;
> + return EFI_INVALID_PARAMETER;
> }
>
> // Cover the entire GCD memory space
> @@ -632,7 +632,7 @@ ArmConfigureMmu (
> } else {
> DEBUG ((EFI_D_ERROR, "ArmConfigureMmu: The MaxAddress 0x%lX is not supported by this MMU configuration.\n", MaxAddress));
> ASSERT (0); // Bigger than 48-bit memory space are not supported
> - return RETURN_UNSUPPORTED;
> + return EFI_UNSUPPORTED;
> }
> } else if (ArmReadCurrentEL () == AARCH64_EL1) {
> // Due to Cortex-A57 erratum #822227 we must set TG1[1] == 1, regardless of EPD1.
> @@ -654,11 +654,11 @@ ArmConfigureMmu (
> } else {
> DEBUG ((EFI_D_ERROR, "ArmConfigureMmu: The MaxAddress 0x%lX is not supported by this MMU configuration.\n", MaxAddress));
> ASSERT (0); // Bigger than 48-bit memory space are not supported
> - return RETURN_UNSUPPORTED;
> + return EFI_UNSUPPORTED;
> }
> } else {
> ASSERT (0); // UEFI is only expected to run at EL2 and EL1, not EL3.
> - return RETURN_UNSUPPORTED;
> + return EFI_UNSUPPORTED;
> }
>
> //
> @@ -680,7 +680,7 @@ ArmConfigureMmu (
> // Allocate pages for translation table
> TranslationTable = AllocatePages (1);
> if (TranslationTable == NULL) {
> - return RETURN_OUT_OF_RESOURCES;
> + return EFI_OUT_OF_RESOURCES;
> }
> // We set TTBR0 just after allocating the table to retrieve its location from the subsequent
> // functions without needing to pass this value across the functions. The MMU is only enabled
> @@ -719,7 +719,7 @@ ArmConfigureMmu (
> DEBUG_CODE_END ();
>
> Status = FillTranslationTable (TranslationTable, MemoryTable);
> - if (RETURN_ERROR (Status)) {
> + if (EFI_ERROR (Status)) {
> goto FREE_TRANSLATION_TABLE;
> }
> MemoryTable++;
> @@ -739,7 +739,7 @@ ArmConfigureMmu (
> ArmEnableDataCache ();
>
> ArmEnableMmu ();
> - return RETURN_SUCCESS;
> + return EFI_SUCCESS;
>
> FREE_TRANSLATION_TABLE:
> FreePages (TranslationTable, 1);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib
2017-03-01 16:31 [PATCH 0/5] ArmPkg, ArmVirtPkg ARM: enable non-executable stack Ard Biesheuvel
2017-03-01 16:31 ` [PATCH 1/5] ArmPkg/ArmMmuLib AARCH64: use correct return type for exported functions Ard Biesheuvel
@ 2017-03-01 16:31 ` Ard Biesheuvel
2017-03-06 16:03 ` Leif Lindholm
2017-03-01 16:31 ` [PATCH 3/5] ArmPkg/ArmMmuLib: remove VirtualMask arg from ArmSetMemoryAttributes Ard Biesheuvel
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-01 16:31 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
... where it belongs, since AARCH64 already keeps it there, and
non DXE users of ArmMmuLib (such as DxeIpl, for the non-executable
stack) may need its functionality as well.
While at it, rename SetMemoryAttributes to ArmSetMemoryAttributes,
and make any functions that are not exported STATIC. Also, replace
an explicit gBS->AllocatePages() call [which is DXE specific] with
MemoryAllocationLib::AllocatePages().
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 368 --------------------
ArmPkg/Drivers/CpuDxe/CpuDxe.h | 14 +-
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
ArmPkg/Include/Library/ArmMmuLib.h | 8 +
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 368 ++++++++++++++++++++
6 files changed, 379 insertions(+), 383 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 6322d301060e..b985dd743f02 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -343,374 +343,6 @@ SyncCacheConfig (
return EFI_SUCCESS;
}
-
-
-EFI_STATUS
-UpdatePageEntries (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
- )
-{
- 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) != 0) {
- 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 we can safely use a switch statement
- if ((Attributes & EFI_MEMORY_UC) != 0) {
- // 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) != 0) {
- // 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) != 0) {
- // 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) != 0) {
- // 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
- }
-
- if ((Attributes & EFI_MEMORY_RO) != 0) {
- 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];
- }
-
- // 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 (VirtualMask != 0) {
- // Make this virtual address point at a physical page
- PageTableEntry &= ~VirtualMask;
- }
-
- if (CurrentPageTableEntry != PageTableEntry) {
- Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
- if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
- // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
- // Note assumes switch(Attributes), not ARMv7 possibilities
- WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
- }
-
- // 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;
-}
-
-
-
-EFI_STATUS
-UpdateSectionEntries (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
- )
-{
- 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 we can safely use a switch statement
- if ((Attributes & EFI_MEMORY_UC) != 0) {
- // 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) != 0) {
- // 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) != 0) {
- // 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) != 0) {
- // 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
- }
-
- if ((Attributes & EFI_MEMORY_RO) != 0) {
- EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
- } else {
- EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
- }
-
- if ((Attributes & EFI_MEMORY_XP) != 0) {
- 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 coverted 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, VirtualMask);
- } else {
- // still a section entry
-
- // mask off appropriate fields
- Descriptor = CurrentDescriptor & ~EntryMask;
-
- // mask in new attributes and/or permissions
- Descriptor |= EntryValue;
- if (VirtualMask != 0) {
- Descriptor &= ~VirtualMask;
- }
-
- if (CurrentDescriptor != Descriptor) {
- Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
- if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
- // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
- // Note assumes switch(Attributes), not ARMv7 possabilities
- WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
- }
-
- // 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
-ConvertSectionToPages (
- IN EFI_PHYSICAL_ADDRESS BaseAddress
- )
-{
- EFI_STATUS Status;
- EFI_PHYSICAL_ADDRESS PageTableAddr;
- 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)
- Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData, 1, &PageTableAddr);
- if (EFI_ERROR(Status)) {
- return Status;
- }
-
- PageTable = (volatile ARM_PAGE_TABLE_ENTRY *)(UINTN)PageTableAddr;
-
- // 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;
- }
-
- // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
- WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)PageTableAddr, TT_DESCRIPTOR_PAGE_SIZE);
-
- // Formulate page table entry, Domain=0, NS=0
- PageTableDescriptor = (((UINTN)PageTableAddr) & 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;
-}
-
-
-
-EFI_STATUS
-SetMemoryAttributes (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
- )
-{
- EFI_STATUS Status;
- UINT64 ChunkLength;
- BOOLEAN FlushTlbs;
-
- 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 | DEBUG_INFO,
- "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
- BaseAddress, ChunkLength, Attributes));
-
- Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
- VirtualMask);
-
- 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 | DEBUG_INFO,
- "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
- BaseAddress, ChunkLength, Attributes));
-
- Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
- VirtualMask);
- }
-
- if (EFI_ERROR (Status)) {
- break;
- }
-
- BaseAddress += ChunkLength;
- Length -= ChunkLength;
- }
-
- if (FlushTlbs) {
- ArmInvalidateTlb ();
- }
- return Status;
-}
-
UINT64
EfiAttributeToArmAttribute (
IN UINT64 EfiAttributes
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
index a46db8d25754..a0f71e69ec09 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
@@ -19,6 +19,7 @@
#include <Uefi.h>
#include <Library/ArmLib.h>
+#include <Library/ArmMmuLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/PcdLib.h>
@@ -112,11 +113,6 @@ SyncCacheConfig (
IN EFI_CPU_ARCH_PROTOCOL *CpuProtocol
);
-EFI_STATUS
-ConvertSectionToPages (
- IN EFI_PHYSICAL_ADDRESS BaseAddress
- );
-
/**
* Publish ARM Processor Data table in UEFI SYSTEM Table.
* @param HobStart Pointer to the beginning of the HOB List from PEI.
@@ -132,14 +128,6 @@ PublishArmProcessorTable(
VOID
);
-EFI_STATUS
-SetMemoryAttributes (
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
- );
-
// The ARM Attributes might be defined on 64-bit (case of the long format description table)
UINT64
EfiAttributeToArmAttribute (
diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
index 0f36a058407a..d0a3fedd3aa7 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
@@ -210,7 +210,7 @@ CpuSetMemoryAttributes (
if (EFI_ERROR (Status) || (RegionArmAttributes != ArmAttributes) ||
((BaseAddress + Length) > (RegionBaseAddress + RegionLength)))
{
- return SetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
+ return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
} else {
return EFI_SUCCESS;
}
diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/ArmMmuLib.h
index c1d43872d548..d3a302fa8125 100644
--- a/ArmPkg/Include/Library/ArmMmuLib.h
+++ b/ArmPkg/Include/Library/ArmMmuLib.h
@@ -62,4 +62,12 @@ ArmReplaceLiveTranslationEntry (
IN UINT64 Value
);
+EFI_STATUS
+ArmSetMemoryAttributes (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN EFI_PHYSICAL_ADDRESS VirtualMask
+ );
+
#endif
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index df170d20a2c2..77f108971f3e 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -447,7 +447,7 @@ GcdAttributeToPageAttribute (
}
EFI_STATUS
-SetMemoryAttributes (
+ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
IN UINT64 Attributes,
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 4b6f4ce392b7..93980d6d12db 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -16,6 +16,7 @@
#include <Uefi.h>
#include <Chipset/ArmV7.h>
#include <Library/BaseMemoryLib.h>
+#include <Library/CacheMaintenanceLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/ArmLib.h>
#include <Library/BaseLib.h>
@@ -36,6 +37,12 @@
#define ID_MMFR0_SHR_IMP_HW_COHERENT 1
#define ID_MMFR0_SHR_IGNORED 0xf
+// First Level Descriptors
+typedef UINT32 ARM_FIRST_LEVEL_DESCRIPTOR;
+
+// Second Level Descriptors
+typedef UINT32 ARM_PAGE_TABLE_ENTRY;
+
UINTN
EFIAPI
ArmReadIdMmfr0 (
@@ -406,6 +413,367 @@ ArmConfigureMmu (
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;
+ }
+
+ // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
+ WriteBackInvalidateDataCacheRange ((VOID *)PageTable, TT_DESCRIPTOR_PAGE_SIZE);
+
+ // 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,
+ IN EFI_PHYSICAL_ADDRESS VirtualMask
+ )
+{
+ 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) != 0) {
+ 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 we can safely use a switch statement
+ if ((Attributes & EFI_MEMORY_UC) != 0) {
+ // 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) != 0) {
+ // 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) != 0) {
+ // 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) != 0) {
+ // 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
+ }
+
+ if ((Attributes & EFI_MEMORY_RO) != 0) {
+ 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];
+ }
+
+ // 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 (VirtualMask != 0) {
+ // Make this virtual address point at a physical page
+ PageTableEntry &= ~VirtualMask;
+ }
+
+ if (CurrentPageTableEntry != PageTableEntry) {
+ Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
+ if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
+ // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
+ // Note assumes switch(Attributes), not ARMv7 possibilities
+ WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
+ }
+
+ // 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,
+ IN EFI_PHYSICAL_ADDRESS VirtualMask
+ )
+{
+ 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 we can safely use a switch statement
+ if ((Attributes & EFI_MEMORY_UC) != 0) {
+ // 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) != 0) {
+ // 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) != 0) {
+ // 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) != 0) {
+ // 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
+ }
+
+ if ((Attributes & EFI_MEMORY_RO) != 0) {
+ EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
+ } else {
+ EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
+ }
+
+ if ((Attributes & EFI_MEMORY_XP) != 0) {
+ 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 coverted 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, VirtualMask);
+ } else {
+ // still a section entry
+
+ // mask off appropriate fields
+ Descriptor = CurrentDescriptor & ~EntryMask;
+
+ // mask in new attributes and/or permissions
+ Descriptor |= EntryValue;
+ if (VirtualMask != 0) {
+ Descriptor &= ~VirtualMask;
+ }
+
+ if (CurrentDescriptor != Descriptor) {
+ Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
+ if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
+ // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
+ // Note assumes switch(Attributes), not ARMv7 possabilities
+ WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
+ }
+
+ // 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,
+ IN EFI_PHYSICAL_ADDRESS VirtualMask
+ )
+{
+ EFI_STATUS Status;
+ UINT64 ChunkLength;
+ BOOLEAN FlushTlbs;
+
+ 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 | DEBUG_INFO,
+ "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
+ BaseAddress, ChunkLength, Attributes));
+
+ Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
+ VirtualMask);
+
+ 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 | DEBUG_INFO,
+ "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
+ BaseAddress, ChunkLength, Attributes));
+
+ Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
+ VirtualMask);
+ }
+
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ BaseAddress += ChunkLength;
+ Length -= ChunkLength;
+ }
+
+ if (FlushTlbs) {
+ ArmInvalidateTlb ();
+ }
+ return Status;
+}
+
RETURN_STATUS
ArmSetMemoryRegionNoExec (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib
2017-03-01 16:31 ` [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib Ard Biesheuvel
@ 2017-03-06 16:03 ` Leif Lindholm
2017-03-06 16:05 ` Ard Biesheuvel
0 siblings, 1 reply; 14+ messages in thread
From: Leif Lindholm @ 2017-03-06 16:03 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek
n Wed, Mar 01, 2017 at 04:31:40PM +0000, Ard Biesheuvel wrote:
> ... where it belongs, since AARCH64 already keeps it there, and
> non DXE users of ArmMmuLib (such as DxeIpl, for the non-executable
> stack) may need its functionality as well.
>
> While at it, rename SetMemoryAttributes to ArmSetMemoryAttributes,
> and make any functions that are not exported STATIC. Also, replace
> an explicit gBS->AllocatePages() call [which is DXE specific] with
> MemoryAllocationLib::AllocatePages().
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 368 --------------------
> ArmPkg/Drivers/CpuDxe/CpuDxe.h | 14 +-
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
> ArmPkg/Include/Library/ArmMmuLib.h | 8 +
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 368 ++++++++++++++++++++
> 6 files changed, 379 insertions(+), 383 deletions(-)
>
> diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> index 6322d301060e..b985dd743f02 100644
> --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> @@ -343,374 +343,6 @@ SyncCacheConfig (
> return EFI_SUCCESS;
> }
>
> -
> -
> -EFI_STATUS
> -UpdatePageEntries (
> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
> - IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> - )
> -{
> - 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) != 0) {
> - 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 we can safely use a switch statement
> - if ((Attributes & EFI_MEMORY_UC) != 0) {
> - // 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) != 0) {
> - // 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) != 0) {
> - // 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) != 0) {
> - // 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
> - }
> -
> - if ((Attributes & EFI_MEMORY_RO) != 0) {
> - 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];
> - }
> -
> - // 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 (VirtualMask != 0) {
> - // Make this virtual address point at a physical page
> - PageTableEntry &= ~VirtualMask;
> - }
> -
> - if (CurrentPageTableEntry != PageTableEntry) {
> - Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
> - if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
> - // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
> - // Note assumes switch(Attributes), not ARMv7 possibilities
> - WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
> - }
> -
> - // 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;
> -}
> -
> -
> -
> -EFI_STATUS
> -UpdateSectionEntries (
> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
> - IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> - )
> -{
> - 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 we can safely use a switch statement
> - if ((Attributes & EFI_MEMORY_UC) != 0) {
> - // 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) != 0) {
> - // 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) != 0) {
> - // 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) != 0) {
> - // 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
> - }
> -
> - if ((Attributes & EFI_MEMORY_RO) != 0) {
> - EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
> - } else {
> - EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
> - }
> -
> - if ((Attributes & EFI_MEMORY_XP) != 0) {
> - 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 coverted 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, VirtualMask);
> - } else {
> - // still a section entry
> -
> - // mask off appropriate fields
> - Descriptor = CurrentDescriptor & ~EntryMask;
> -
> - // mask in new attributes and/or permissions
> - Descriptor |= EntryValue;
> - if (VirtualMask != 0) {
> - Descriptor &= ~VirtualMask;
> - }
> -
> - if (CurrentDescriptor != Descriptor) {
> - Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> - if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
> - // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
> - // Note assumes switch(Attributes), not ARMv7 possabilities
> - WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
> - }
> -
> - // 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
> -ConvertSectionToPages (
> - IN EFI_PHYSICAL_ADDRESS BaseAddress
> - )
> -{
> - EFI_STATUS Status;
> - EFI_PHYSICAL_ADDRESS PageTableAddr;
> - 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)
> - Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData, 1, &PageTableAddr);
> - if (EFI_ERROR(Status)) {
> - return Status;
> - }
> -
> - PageTable = (volatile ARM_PAGE_TABLE_ENTRY *)(UINTN)PageTableAddr;
> -
> - // 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;
> - }
> -
> - // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
> - WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)PageTableAddr, TT_DESCRIPTOR_PAGE_SIZE);
> -
> - // Formulate page table entry, Domain=0, NS=0
> - PageTableDescriptor = (((UINTN)PageTableAddr) & 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;
> -}
> -
> -
> -
> -EFI_STATUS
> -SetMemoryAttributes (
> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
> - IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> - )
> -{
> - EFI_STATUS Status;
> - UINT64 ChunkLength;
> - BOOLEAN FlushTlbs;
> -
> - 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 | DEBUG_INFO,
> - "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
> - BaseAddress, ChunkLength, Attributes));
> -
> - Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
> - VirtualMask);
> -
> - 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 | DEBUG_INFO,
> - "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
> - BaseAddress, ChunkLength, Attributes));
> -
> - Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
> - VirtualMask);
> - }
> -
> - if (EFI_ERROR (Status)) {
> - break;
> - }
> -
> - BaseAddress += ChunkLength;
> - Length -= ChunkLength;
> - }
> -
> - if (FlushTlbs) {
> - ArmInvalidateTlb ();
> - }
> - return Status;
> -}
> -
> UINT64
> EfiAttributeToArmAttribute (
> IN UINT64 EfiAttributes
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> index a46db8d25754..a0f71e69ec09 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> @@ -19,6 +19,7 @@
> #include <Uefi.h>
>
> #include <Library/ArmLib.h>
> +#include <Library/ArmMmuLib.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> #include <Library/PcdLib.h>
> @@ -112,11 +113,6 @@ SyncCacheConfig (
> IN EFI_CPU_ARCH_PROTOCOL *CpuProtocol
> );
>
> -EFI_STATUS
> -ConvertSectionToPages (
> - IN EFI_PHYSICAL_ADDRESS BaseAddress
> - );
> -
> /**
> * Publish ARM Processor Data table in UEFI SYSTEM Table.
> * @param HobStart Pointer to the beginning of the HOB List from PEI.
> @@ -132,14 +128,6 @@ PublishArmProcessorTable(
> VOID
> );
>
> -EFI_STATUS
> -SetMemoryAttributes (
> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
> - IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> - );
> -
> // The ARM Attributes might be defined on 64-bit (case of the long format description table)
> UINT64
> EfiAttributeToArmAttribute (
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> index 0f36a058407a..d0a3fedd3aa7 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> @@ -210,7 +210,7 @@ CpuSetMemoryAttributes (
> if (EFI_ERROR (Status) || (RegionArmAttributes != ArmAttributes) ||
> ((BaseAddress + Length) > (RegionBaseAddress + RegionLength)))
> {
> - return SetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
> + return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
> } else {
> return EFI_SUCCESS;
> }
> diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/ArmMmuLib.h
> index c1d43872d548..d3a302fa8125 100644
> --- a/ArmPkg/Include/Library/ArmMmuLib.h
> +++ b/ArmPkg/Include/Library/ArmMmuLib.h
> @@ -62,4 +62,12 @@ ArmReplaceLiveTranslationEntry (
> IN UINT64 Value
> );
>
> +EFI_STATUS
> +ArmSetMemoryAttributes (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes,
> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> + );
> +
> #endif
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index df170d20a2c2..77f108971f3e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -447,7 +447,7 @@ GcdAttributeToPageAttribute (
> }
>
> EFI_STATUS
> -SetMemoryAttributes (
> +ArmSetMemoryAttributes (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> IN UINT64 Attributes,
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index 4b6f4ce392b7..93980d6d12db 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -16,6 +16,7 @@
> #include <Uefi.h>
> #include <Chipset/ArmV7.h>
> #include <Library/BaseMemoryLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> #include <Library/MemoryAllocationLib.h>
> #include <Library/ArmLib.h>
> #include <Library/BaseLib.h>
> @@ -36,6 +37,12 @@
> #define ID_MMFR0_SHR_IMP_HW_COHERENT 1
> #define ID_MMFR0_SHR_IGNORED 0xf
>
> +// First Level Descriptors
> +typedef UINT32 ARM_FIRST_LEVEL_DESCRIPTOR;
> +
> +// Second Level Descriptors
> +typedef UINT32 ARM_PAGE_TABLE_ENTRY;
> +
Copied from ArmPkg/Drivers/CpuDxe/Arm/Mmu.c, but not deleted there.
Can it be, or can it be moved out into a header somewhere?
No other comments.
/
Leif
> UINTN
> EFIAPI
> ArmReadIdMmfr0 (
> @@ -406,6 +413,367 @@ ArmConfigureMmu (
> 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;
> + }
> +
> + // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
> + WriteBackInvalidateDataCacheRange ((VOID *)PageTable, TT_DESCRIPTOR_PAGE_SIZE);
> +
> + // 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,
> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> + )
> +{
> + 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) != 0) {
> + 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 we can safely use a switch statement
> + if ((Attributes & EFI_MEMORY_UC) != 0) {
> + // 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) != 0) {
> + // 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) != 0) {
> + // 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) != 0) {
> + // 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
> + }
> +
> + if ((Attributes & EFI_MEMORY_RO) != 0) {
> + 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];
> + }
> +
> + // 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 (VirtualMask != 0) {
> + // Make this virtual address point at a physical page
> + PageTableEntry &= ~VirtualMask;
> + }
> +
> + if (CurrentPageTableEntry != PageTableEntry) {
> + Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
> + if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
> + // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
> + // Note assumes switch(Attributes), not ARMv7 possibilities
> + WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
> + }
> +
> + // 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,
> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> + )
> +{
> + 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 we can safely use a switch statement
> + if ((Attributes & EFI_MEMORY_UC) != 0) {
> + // 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) != 0) {
> + // 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) != 0) {
> + // 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) != 0) {
> + // 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
> + }
> +
> + if ((Attributes & EFI_MEMORY_RO) != 0) {
> + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
> + } else {
> + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
> + }
> +
> + if ((Attributes & EFI_MEMORY_XP) != 0) {
> + 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 coverted 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, VirtualMask);
> + } else {
> + // still a section entry
> +
> + // mask off appropriate fields
> + Descriptor = CurrentDescriptor & ~EntryMask;
> +
> + // mask in new attributes and/or permissions
> + Descriptor |= EntryValue;
> + if (VirtualMask != 0) {
> + Descriptor &= ~VirtualMask;
> + }
> +
> + if (CurrentDescriptor != Descriptor) {
> + Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> + if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
> + // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
> + // Note assumes switch(Attributes), not ARMv7 possabilities
> + WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
> + }
> +
> + // 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,
> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> + )
> +{
> + EFI_STATUS Status;
> + UINT64 ChunkLength;
> + BOOLEAN FlushTlbs;
> +
> + 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 | DEBUG_INFO,
> + "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
> + BaseAddress, ChunkLength, Attributes));
> +
> + Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
> + VirtualMask);
> +
> + 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 | DEBUG_INFO,
> + "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
> + BaseAddress, ChunkLength, Attributes));
> +
> + Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
> + VirtualMask);
> + }
> +
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> +
> + BaseAddress += ChunkLength;
> + Length -= ChunkLength;
> + }
> +
> + if (FlushTlbs) {
> + ArmInvalidateTlb ();
> + }
> + return Status;
> +}
> +
> RETURN_STATUS
> ArmSetMemoryRegionNoExec (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib
2017-03-06 16:03 ` Leif Lindholm
@ 2017-03-06 16:05 ` Ard Biesheuvel
2017-03-06 16:21 ` Leif Lindholm
0 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-06 16:05 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Laszlo Ersek
On 6 March 2017 at 17:03, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> n Wed, Mar 01, 2017 at 04:31:40PM +0000, Ard Biesheuvel wrote:
>> ... where it belongs, since AARCH64 already keeps it there, and
>> non DXE users of ArmMmuLib (such as DxeIpl, for the non-executable
>> stack) may need its functionality as well.
>>
>> While at it, rename SetMemoryAttributes to ArmSetMemoryAttributes,
>> and make any functions that are not exported STATIC. Also, replace
>> an explicit gBS->AllocatePages() call [which is DXE specific] with
>> MemoryAllocationLib::AllocatePages().
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 368 --------------------
>> ArmPkg/Drivers/CpuDxe/CpuDxe.h | 14 +-
>> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
>> ArmPkg/Include/Library/ArmMmuLib.h | 8 +
>> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-
>> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 368 ++++++++++++++++++++
>> 6 files changed, 379 insertions(+), 383 deletions(-)
>>
>> diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
>> index 6322d301060e..b985dd743f02 100644
>> --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
>> +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
>> @@ -343,374 +343,6 @@ SyncCacheConfig (
>> return EFI_SUCCESS;
>> }
>>
>> -
>> -
>> -EFI_STATUS
>> -UpdatePageEntries (
>> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> - IN UINT64 Length,
>> - IN UINT64 Attributes,
>> - IN EFI_PHYSICAL_ADDRESS VirtualMask
>> - )
>> -{
>> - 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) != 0) {
>> - 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 we can safely use a switch statement
>> - if ((Attributes & EFI_MEMORY_UC) != 0) {
>> - // 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) != 0) {
>> - // 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) != 0) {
>> - // 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) != 0) {
>> - // 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
>> - }
>> -
>> - if ((Attributes & EFI_MEMORY_RO) != 0) {
>> - 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];
>> - }
>> -
>> - // 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 (VirtualMask != 0) {
>> - // Make this virtual address point at a physical page
>> - PageTableEntry &= ~VirtualMask;
>> - }
>> -
>> - if (CurrentPageTableEntry != PageTableEntry) {
>> - Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
>> - if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
>> - // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
>> - // Note assumes switch(Attributes), not ARMv7 possibilities
>> - WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
>> - }
>> -
>> - // 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;
>> -}
>> -
>> -
>> -
>> -EFI_STATUS
>> -UpdateSectionEntries (
>> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> - IN UINT64 Length,
>> - IN UINT64 Attributes,
>> - IN EFI_PHYSICAL_ADDRESS VirtualMask
>> - )
>> -{
>> - 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 we can safely use a switch statement
>> - if ((Attributes & EFI_MEMORY_UC) != 0) {
>> - // 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) != 0) {
>> - // 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) != 0) {
>> - // 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) != 0) {
>> - // 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
>> - }
>> -
>> - if ((Attributes & EFI_MEMORY_RO) != 0) {
>> - EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
>> - } else {
>> - EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
>> - }
>> -
>> - if ((Attributes & EFI_MEMORY_XP) != 0) {
>> - 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 coverted 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, VirtualMask);
>> - } else {
>> - // still a section entry
>> -
>> - // mask off appropriate fields
>> - Descriptor = CurrentDescriptor & ~EntryMask;
>> -
>> - // mask in new attributes and/or permissions
>> - Descriptor |= EntryValue;
>> - if (VirtualMask != 0) {
>> - Descriptor &= ~VirtualMask;
>> - }
>> -
>> - if (CurrentDescriptor != Descriptor) {
>> - Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
>> - if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
>> - // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
>> - // Note assumes switch(Attributes), not ARMv7 possabilities
>> - WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
>> - }
>> -
>> - // 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
>> -ConvertSectionToPages (
>> - IN EFI_PHYSICAL_ADDRESS BaseAddress
>> - )
>> -{
>> - EFI_STATUS Status;
>> - EFI_PHYSICAL_ADDRESS PageTableAddr;
>> - 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)
>> - Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData, 1, &PageTableAddr);
>> - if (EFI_ERROR(Status)) {
>> - return Status;
>> - }
>> -
>> - PageTable = (volatile ARM_PAGE_TABLE_ENTRY *)(UINTN)PageTableAddr;
>> -
>> - // 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;
>> - }
>> -
>> - // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
>> - WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)PageTableAddr, TT_DESCRIPTOR_PAGE_SIZE);
>> -
>> - // Formulate page table entry, Domain=0, NS=0
>> - PageTableDescriptor = (((UINTN)PageTableAddr) & 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;
>> -}
>> -
>> -
>> -
>> -EFI_STATUS
>> -SetMemoryAttributes (
>> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> - IN UINT64 Length,
>> - IN UINT64 Attributes,
>> - IN EFI_PHYSICAL_ADDRESS VirtualMask
>> - )
>> -{
>> - EFI_STATUS Status;
>> - UINT64 ChunkLength;
>> - BOOLEAN FlushTlbs;
>> -
>> - 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 | DEBUG_INFO,
>> - "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
>> - BaseAddress, ChunkLength, Attributes));
>> -
>> - Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
>> - VirtualMask);
>> -
>> - 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 | DEBUG_INFO,
>> - "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
>> - BaseAddress, ChunkLength, Attributes));
>> -
>> - Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
>> - VirtualMask);
>> - }
>> -
>> - if (EFI_ERROR (Status)) {
>> - break;
>> - }
>> -
>> - BaseAddress += ChunkLength;
>> - Length -= ChunkLength;
>> - }
>> -
>> - if (FlushTlbs) {
>> - ArmInvalidateTlb ();
>> - }
>> - return Status;
>> -}
>> -
>> UINT64
>> EfiAttributeToArmAttribute (
>> IN UINT64 EfiAttributes
>> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
>> index a46db8d25754..a0f71e69ec09 100644
>> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
>> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
>> @@ -19,6 +19,7 @@
>> #include <Uefi.h>
>>
>> #include <Library/ArmLib.h>
>> +#include <Library/ArmMmuLib.h>
>> #include <Library/BaseMemoryLib.h>
>> #include <Library/DebugLib.h>
>> #include <Library/PcdLib.h>
>> @@ -112,11 +113,6 @@ SyncCacheConfig (
>> IN EFI_CPU_ARCH_PROTOCOL *CpuProtocol
>> );
>>
>> -EFI_STATUS
>> -ConvertSectionToPages (
>> - IN EFI_PHYSICAL_ADDRESS BaseAddress
>> - );
>> -
>> /**
>> * Publish ARM Processor Data table in UEFI SYSTEM Table.
>> * @param HobStart Pointer to the beginning of the HOB List from PEI.
>> @@ -132,14 +128,6 @@ PublishArmProcessorTable(
>> VOID
>> );
>>
>> -EFI_STATUS
>> -SetMemoryAttributes (
>> - IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> - IN UINT64 Length,
>> - IN UINT64 Attributes,
>> - IN EFI_PHYSICAL_ADDRESS VirtualMask
>> - );
>> -
>> // The ARM Attributes might be defined on 64-bit (case of the long format description table)
>> UINT64
>> EfiAttributeToArmAttribute (
>> diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
>> index 0f36a058407a..d0a3fedd3aa7 100644
>> --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
>> +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
>> @@ -210,7 +210,7 @@ CpuSetMemoryAttributes (
>> if (EFI_ERROR (Status) || (RegionArmAttributes != ArmAttributes) ||
>> ((BaseAddress + Length) > (RegionBaseAddress + RegionLength)))
>> {
>> - return SetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
>> + return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
>> } else {
>> return EFI_SUCCESS;
>> }
>> diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/ArmMmuLib.h
>> index c1d43872d548..d3a302fa8125 100644
>> --- a/ArmPkg/Include/Library/ArmMmuLib.h
>> +++ b/ArmPkg/Include/Library/ArmMmuLib.h
>> @@ -62,4 +62,12 @@ ArmReplaceLiveTranslationEntry (
>> IN UINT64 Value
>> );
>>
>> +EFI_STATUS
>> +ArmSetMemoryAttributes (
>> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> + IN UINT64 Length,
>> + IN UINT64 Attributes,
>> + IN EFI_PHYSICAL_ADDRESS VirtualMask
>> + );
>> +
>> #endif
>> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> index df170d20a2c2..77f108971f3e 100644
>> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> @@ -447,7 +447,7 @@ GcdAttributeToPageAttribute (
>> }
>>
>> EFI_STATUS
>> -SetMemoryAttributes (
>> +ArmSetMemoryAttributes (
>> IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> IN UINT64 Length,
>> IN UINT64 Attributes,
>> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
>> index 4b6f4ce392b7..93980d6d12db 100644
>> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
>> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
>> @@ -16,6 +16,7 @@
>> #include <Uefi.h>
>> #include <Chipset/ArmV7.h>
>> #include <Library/BaseMemoryLib.h>
>> +#include <Library/CacheMaintenanceLib.h>
>> #include <Library/MemoryAllocationLib.h>
>> #include <Library/ArmLib.h>
>> #include <Library/BaseLib.h>
>> @@ -36,6 +37,12 @@
>> #define ID_MMFR0_SHR_IMP_HW_COHERENT 1
>> #define ID_MMFR0_SHR_IGNORED 0xf
>>
>> +// First Level Descriptors
>> +typedef UINT32 ARM_FIRST_LEVEL_DESCRIPTOR;
>> +
>> +// Second Level Descriptors
>> +typedef UINT32 ARM_PAGE_TABLE_ENTRY;
>> +
>
> Copied from ArmPkg/Drivers/CpuDxe/Arm/Mmu.c, but not deleted there.
> Can it be, or can it be moved out into a header somewhere?
>
> No other comments.
>
It is used in both places, so I'd need to put in in a header file
ArmPkg/Include/Chipset/ArmV7Mmu.h comes to mind ...
>> UINTN
>> EFIAPI
>> ArmReadIdMmfr0 (
>> @@ -406,6 +413,367 @@ ArmConfigureMmu (
>> 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;
>> + }
>> +
>> + // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
>> + WriteBackInvalidateDataCacheRange ((VOID *)PageTable, TT_DESCRIPTOR_PAGE_SIZE);
>> +
>> + // 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,
>> + IN EFI_PHYSICAL_ADDRESS VirtualMask
>> + )
>> +{
>> + 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) != 0) {
>> + 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 we can safely use a switch statement
>> + if ((Attributes & EFI_MEMORY_UC) != 0) {
>> + // 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) != 0) {
>> + // 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) != 0) {
>> + // 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) != 0) {
>> + // 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
>> + }
>> +
>> + if ((Attributes & EFI_MEMORY_RO) != 0) {
>> + 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];
>> + }
>> +
>> + // 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 (VirtualMask != 0) {
>> + // Make this virtual address point at a physical page
>> + PageTableEntry &= ~VirtualMask;
>> + }
>> +
>> + if (CurrentPageTableEntry != PageTableEntry) {
>> + Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
>> + if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
>> + // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
>> + // Note assumes switch(Attributes), not ARMv7 possibilities
>> + WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
>> + }
>> +
>> + // 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,
>> + IN EFI_PHYSICAL_ADDRESS VirtualMask
>> + )
>> +{
>> + 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 we can safely use a switch statement
>> + if ((Attributes & EFI_MEMORY_UC) != 0) {
>> + // 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) != 0) {
>> + // 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) != 0) {
>> + // 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) != 0) {
>> + // 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
>> + }
>> +
>> + if ((Attributes & EFI_MEMORY_RO) != 0) {
>> + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
>> + } else {
>> + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
>> + }
>> +
>> + if ((Attributes & EFI_MEMORY_XP) != 0) {
>> + 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 coverted 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, VirtualMask);
>> + } else {
>> + // still a section entry
>> +
>> + // mask off appropriate fields
>> + Descriptor = CurrentDescriptor & ~EntryMask;
>> +
>> + // mask in new attributes and/or permissions
>> + Descriptor |= EntryValue;
>> + if (VirtualMask != 0) {
>> + Descriptor &= ~VirtualMask;
>> + }
>> +
>> + if (CurrentDescriptor != Descriptor) {
>> + Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
>> + if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
>> + // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
>> + // Note assumes switch(Attributes), not ARMv7 possabilities
>> + WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
>> + }
>> +
>> + // 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,
>> + IN EFI_PHYSICAL_ADDRESS VirtualMask
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UINT64 ChunkLength;
>> + BOOLEAN FlushTlbs;
>> +
>> + 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 | DEBUG_INFO,
>> + "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
>> + BaseAddress, ChunkLength, Attributes));
>> +
>> + Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
>> + VirtualMask);
>> +
>> + 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 | DEBUG_INFO,
>> + "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
>> + BaseAddress, ChunkLength, Attributes));
>> +
>> + Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
>> + VirtualMask);
>> + }
>> +
>> + if (EFI_ERROR (Status)) {
>> + break;
>> + }
>> +
>> + BaseAddress += ChunkLength;
>> + Length -= ChunkLength;
>> + }
>> +
>> + if (FlushTlbs) {
>> + ArmInvalidateTlb ();
>> + }
>> + return Status;
>> +}
>> +
>> RETURN_STATUS
>> ArmSetMemoryRegionNoExec (
>> IN EFI_PHYSICAL_ADDRESS BaseAddress,
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib
2017-03-06 16:05 ` Ard Biesheuvel
@ 2017-03-06 16:21 ` Leif Lindholm
0 siblings, 0 replies; 14+ messages in thread
From: Leif Lindholm @ 2017-03-06 16:21 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Laszlo Ersek
On Mon, Mar 06, 2017 at 05:05:58PM +0100, Ard Biesheuvel wrote:
> >> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> >> index 4b6f4ce392b7..93980d6d12db 100644
> >> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> >> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> >> @@ -16,6 +16,7 @@
> >> #include <Uefi.h>
> >> #include <Chipset/ArmV7.h>
> >> #include <Library/BaseMemoryLib.h>
> >> +#include <Library/CacheMaintenanceLib.h>
> >> #include <Library/MemoryAllocationLib.h>
> >> #include <Library/ArmLib.h>
> >> #include <Library/BaseLib.h>
> >> @@ -36,6 +37,12 @@
> >> #define ID_MMFR0_SHR_IMP_HW_COHERENT 1
> >> #define ID_MMFR0_SHR_IGNORED 0xf
> >>
> >> +// First Level Descriptors
> >> +typedef UINT32 ARM_FIRST_LEVEL_DESCRIPTOR;
> >> +
> >> +// Second Level Descriptors
> >> +typedef UINT32 ARM_PAGE_TABLE_ENTRY;
> >> +
> >
> > Copied from ArmPkg/Drivers/CpuDxe/Arm/Mmu.c, but not deleted there.
> > Can it be, or can it be moved out into a header somewhere?
> >
> > No other comments.
> >
>
> It is used in both places, so I'd need to put in in a header file
>
> ArmPkg/Include/Chipset/ArmV7Mmu.h comes to mind ...
Works for me.
If you fold that in:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
/
Leif
> >> UINTN
> >> EFIAPI
> >> ArmReadIdMmfr0 (
> >> @@ -406,6 +413,367 @@ ArmConfigureMmu (
> >> 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;
> >> + }
> >> +
> >> + // Flush d-cache so descriptors make it back to uncached memory for subsequent table walks
> >> + WriteBackInvalidateDataCacheRange ((VOID *)PageTable, TT_DESCRIPTOR_PAGE_SIZE);
> >> +
> >> + // 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,
> >> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> >> + )
> >> +{
> >> + 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) != 0) {
> >> + 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 we can safely use a switch statement
> >> + if ((Attributes & EFI_MEMORY_UC) != 0) {
> >> + // 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) != 0) {
> >> + // 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) != 0) {
> >> + // 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) != 0) {
> >> + // 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
> >> + }
> >> +
> >> + if ((Attributes & EFI_MEMORY_RO) != 0) {
> >> + 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];
> >> + }
> >> +
> >> + // 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 (VirtualMask != 0) {
> >> + // Make this virtual address point at a physical page
> >> + PageTableEntry &= ~VirtualMask;
> >> + }
> >> +
> >> + if (CurrentPageTableEntry != PageTableEntry) {
> >> + Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
> >> + if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
> >> + // The current section mapping is cacheable so Clean/Invalidate the MVA of the page
> >> + // Note assumes switch(Attributes), not ARMv7 possibilities
> >> + WriteBackInvalidateDataCacheRange (Mva, TT_DESCRIPTOR_PAGE_SIZE);
> >> + }
> >> +
> >> + // 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,
> >> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> >> + )
> >> +{
> >> + 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 we can safely use a switch statement
> >> + if ((Attributes & EFI_MEMORY_UC) != 0) {
> >> + // 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) != 0) {
> >> + // 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) != 0) {
> >> + // 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) != 0) {
> >> + // 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
> >> + }
> >> +
> >> + if ((Attributes & EFI_MEMORY_RO) != 0) {
> >> + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
> >> + } else {
> >> + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
> >> + }
> >> +
> >> + if ((Attributes & EFI_MEMORY_XP) != 0) {
> >> + 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 coverted 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, VirtualMask);
> >> + } else {
> >> + // still a section entry
> >> +
> >> + // mask off appropriate fields
> >> + Descriptor = CurrentDescriptor & ~EntryMask;
> >> +
> >> + // mask in new attributes and/or permissions
> >> + Descriptor |= EntryValue;
> >> + if (VirtualMask != 0) {
> >> + Descriptor &= ~VirtualMask;
> >> + }
> >> +
> >> + if (CurrentDescriptor != Descriptor) {
> >> + Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> >> + if ((CurrentDescriptor & TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) == TT_DESCRIPTOR_SECTION_CACHEABLE_MASK) {
> >> + // The current section mapping is cacheable so Clean/Invalidate the MVA of the section
> >> + // Note assumes switch(Attributes), not ARMv7 possabilities
> >> + WriteBackInvalidateDataCacheRange (Mva, SIZE_1MB);
> >> + }
> >> +
> >> + // 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,
> >> + IN EFI_PHYSICAL_ADDRESS VirtualMask
> >> + )
> >> +{
> >> + EFI_STATUS Status;
> >> + UINT64 ChunkLength;
> >> + BOOLEAN FlushTlbs;
> >> +
> >> + 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 | DEBUG_INFO,
> >> + "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
> >> + BaseAddress, ChunkLength, Attributes));
> >> +
> >> + Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
> >> + VirtualMask);
> >> +
> >> + 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 | DEBUG_INFO,
> >> + "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
> >> + BaseAddress, ChunkLength, Attributes));
> >> +
> >> + Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
> >> + VirtualMask);
> >> + }
> >> +
> >> + if (EFI_ERROR (Status)) {
> >> + break;
> >> + }
> >> +
> >> + BaseAddress += ChunkLength;
> >> + Length -= ChunkLength;
> >> + }
> >> +
> >> + if (FlushTlbs) {
> >> + ArmInvalidateTlb ();
> >> + }
> >> + return Status;
> >> +}
> >> +
> >> RETURN_STATUS
> >> ArmSetMemoryRegionNoExec (
> >> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> >> --
> >> 2.7.4
> >>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/5] ArmPkg/ArmMmuLib: remove VirtualMask arg from ArmSetMemoryAttributes
2017-03-01 16:31 [PATCH 0/5] ArmPkg, ArmVirtPkg ARM: enable non-executable stack Ard Biesheuvel
2017-03-01 16:31 ` [PATCH 1/5] ArmPkg/ArmMmuLib AARCH64: use correct return type for exported functions Ard Biesheuvel
2017-03-01 16:31 ` [PATCH 2/5] ArmPkg: move ARM version of SetMemoryAttributes to ArmMmuLib Ard Biesheuvel
@ 2017-03-01 16:31 ` Ard Biesheuvel
2017-03-06 16:06 ` Leif Lindholm
2017-03-01 16:31 ` [PATCH 4/5] ArmPkg/ArmMmuLib ARM: implement memory permission control routines Ard Biesheuvel
2017-03-01 16:31 ` [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms Ard Biesheuvel
4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-01 16:31 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
We no longer make use of the ArmMmuLib 'feature' to create aliased
memory ranges with mismatched attributes, and in fact, it was only
wired up in the ARM version to begin with.
So remove the VirtualMask argument from ArmSetMemoryAttributes()'s
prototype, and remove the dead code that referred to it.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
ArmPkg/Include/Library/ArmMmuLib.h | 3 +--
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 3 +--
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 25 +++++---------------
4 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
index d0a3fedd3aa7..8150486217cf 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
@@ -210,7 +210,7 @@ CpuSetMemoryAttributes (
if (EFI_ERROR (Status) || (RegionArmAttributes != ArmAttributes) ||
((BaseAddress + Length) > (RegionBaseAddress + RegionLength)))
{
- return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
+ return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes);
} else {
return EFI_SUCCESS;
}
diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/ArmMmuLib.h
index d3a302fa8125..fb7fd006417c 100644
--- a/ArmPkg/Include/Library/ArmMmuLib.h
+++ b/ArmPkg/Include/Library/ArmMmuLib.h
@@ -66,8 +66,7 @@ EFI_STATUS
ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
+ IN UINT64 Attributes
);
#endif
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 77f108971f3e..8bd1c6fad95f 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -450,8 +450,7 @@ EFI_STATUS
ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
+ IN UINT64 Attributes
)
{
EFI_STATUS Status;
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 93980d6d12db..1112660b434e 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -469,8 +469,7 @@ EFI_STATUS
UpdatePageEntries (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
+ IN UINT64 Attributes
)
{
EFI_STATUS Status;
@@ -575,11 +574,6 @@ UpdatePageEntries (
// Mask in new attributes and/or permissions
PageTableEntry |= EntryValue;
- if (VirtualMask != 0) {
- // Make this virtual address point at a physical page
- PageTableEntry &= ~VirtualMask;
- }
-
if (CurrentPageTableEntry != PageTableEntry) {
Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
@@ -606,8 +600,7 @@ EFI_STATUS
UpdateSectionEntries (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
+ IN UINT64 Attributes
)
{
EFI_STATUS Status = EFI_SUCCESS;
@@ -680,7 +673,7 @@ UpdateSectionEntries (
// has this descriptor already been coverted 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, VirtualMask);
+ Status = UpdatePageEntries ((FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT, TT_DESCRIPTOR_SECTION_SIZE, Attributes);
} else {
// still a section entry
@@ -689,9 +682,6 @@ UpdateSectionEntries (
// mask in new attributes and/or permissions
Descriptor |= EntryValue;
- if (VirtualMask != 0) {
- Descriptor &= ~VirtualMask;
- }
if (CurrentDescriptor != Descriptor) {
Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
@@ -717,8 +707,7 @@ EFI_STATUS
ArmSetMemoryAttributes (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
- IN UINT64 Attributes,
- IN EFI_PHYSICAL_ADDRESS VirtualMask
+ IN UINT64 Attributes
)
{
EFI_STATUS Status;
@@ -736,8 +725,7 @@ ArmSetMemoryAttributes (
"SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
BaseAddress, ChunkLength, Attributes));
- Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
- VirtualMask);
+ Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes);
FlushTlbs = TRUE;
} else {
@@ -756,8 +744,7 @@ ArmSetMemoryAttributes (
"SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
BaseAddress, ChunkLength, Attributes));
- Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
- VirtualMask);
+ Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes);
}
if (EFI_ERROR (Status)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/5] ArmPkg/ArmMmuLib: remove VirtualMask arg from ArmSetMemoryAttributes
2017-03-01 16:31 ` [PATCH 3/5] ArmPkg/ArmMmuLib: remove VirtualMask arg from ArmSetMemoryAttributes Ard Biesheuvel
@ 2017-03-06 16:06 ` Leif Lindholm
0 siblings, 0 replies; 14+ messages in thread
From: Leif Lindholm @ 2017-03-06 16:06 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek
On Wed, Mar 01, 2017 at 04:31:41PM +0000, Ard Biesheuvel wrote:
> We no longer make use of the ArmMmuLib 'feature' to create aliased
> memory ranges with mismatched attributes, and in fact, it was only
> wired up in the ARM version to begin with.
>
> So remove the VirtualMask argument from ArmSetMemoryAttributes()'s
> prototype, and remove the dead code that referred to it.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 2 +-
> ArmPkg/Include/Library/ArmMmuLib.h | 3 +--
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 3 +--
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 25 +++++---------------
> 4 files changed, 9 insertions(+), 24 deletions(-)
>
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> index d0a3fedd3aa7..8150486217cf 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> @@ -210,7 +210,7 @@ CpuSetMemoryAttributes (
> if (EFI_ERROR (Status) || (RegionArmAttributes != ArmAttributes) ||
> ((BaseAddress + Length) > (RegionBaseAddress + RegionLength)))
> {
> - return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes, 0);
> + return ArmSetMemoryAttributes (BaseAddress, Length, EfiAttributes);
> } else {
> return EFI_SUCCESS;
> }
> diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/ArmMmuLib.h
> index d3a302fa8125..fb7fd006417c 100644
> --- a/ArmPkg/Include/Library/ArmMmuLib.h
> +++ b/ArmPkg/Include/Library/ArmMmuLib.h
> @@ -66,8 +66,7 @@ EFI_STATUS
> ArmSetMemoryAttributes (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> + IN UINT64 Attributes
> );
>
> #endif
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 77f108971f3e..8bd1c6fad95f 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -450,8 +450,7 @@ EFI_STATUS
> ArmSetMemoryAttributes (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> + IN UINT64 Attributes
> )
> {
> EFI_STATUS Status;
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index 93980d6d12db..1112660b434e 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -469,8 +469,7 @@ EFI_STATUS
> UpdatePageEntries (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> + IN UINT64 Attributes
> )
> {
> EFI_STATUS Status;
> @@ -575,11 +574,6 @@ UpdatePageEntries (
> // Mask in new attributes and/or permissions
> PageTableEntry |= EntryValue;
>
> - if (VirtualMask != 0) {
> - // Make this virtual address point at a physical page
> - PageTableEntry &= ~VirtualMask;
> - }
> -
> if (CurrentPageTableEntry != PageTableEntry) {
> Mva = (VOID *)(UINTN)((((UINTN)FirstLevelIdx) << TT_DESCRIPTOR_SECTION_BASE_SHIFT) + (PageTableIndex << TT_DESCRIPTOR_PAGE_BASE_SHIFT));
> if ((CurrentPageTableEntry & TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) == TT_DESCRIPTOR_PAGE_CACHEABLE_MASK) {
> @@ -606,8 +600,7 @@ EFI_STATUS
> UpdateSectionEntries (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> + IN UINT64 Attributes
> )
> {
> EFI_STATUS Status = EFI_SUCCESS;
> @@ -680,7 +673,7 @@ UpdateSectionEntries (
> // has this descriptor already been coverted 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, VirtualMask);
> + Status = UpdatePageEntries ((FirstLevelIdx + i) << TT_DESCRIPTOR_SECTION_BASE_SHIFT, TT_DESCRIPTOR_SECTION_SIZE, Attributes);
> } else {
> // still a section entry
>
> @@ -689,9 +682,6 @@ UpdateSectionEntries (
>
> // mask in new attributes and/or permissions
> Descriptor |= EntryValue;
> - if (VirtualMask != 0) {
> - Descriptor &= ~VirtualMask;
> - }
>
> if (CurrentDescriptor != Descriptor) {
> Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << TT_DESCRIPTOR_SECTION_BASE_SHIFT);
> @@ -717,8 +707,7 @@ EFI_STATUS
> ArmSetMemoryAttributes (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length,
> - IN UINT64 Attributes,
> - IN EFI_PHYSICAL_ADDRESS VirtualMask
> + IN UINT64 Attributes
> )
> {
> EFI_STATUS Status;
> @@ -736,8 +725,7 @@ ArmSetMemoryAttributes (
> "SetMemoryAttributes(): MMU section 0x%lx length 0x%lx to %lx\n",
> BaseAddress, ChunkLength, Attributes));
>
> - Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes,
> - VirtualMask);
> + Status = UpdateSectionEntries (BaseAddress, ChunkLength, Attributes);
>
> FlushTlbs = TRUE;
> } else {
> @@ -756,8 +744,7 @@ ArmSetMemoryAttributes (
> "SetMemoryAttributes(): MMU page 0x%lx length 0x%lx to %lx\n",
> BaseAddress, ChunkLength, Attributes));
>
> - Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes,
> - VirtualMask);
> + Status = UpdatePageEntries (BaseAddress, ChunkLength, Attributes);
> }
>
> if (EFI_ERROR (Status)) {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/5] ArmPkg/ArmMmuLib ARM: implement memory permission control routines
2017-03-01 16:31 [PATCH 0/5] ArmPkg, ArmVirtPkg ARM: enable non-executable stack Ard Biesheuvel
` (2 preceding siblings ...)
2017-03-01 16:31 ` [PATCH 3/5] ArmPkg/ArmMmuLib: remove VirtualMask arg from ArmSetMemoryAttributes Ard Biesheuvel
@ 2017-03-01 16:31 ` Ard Biesheuvel
2017-03-06 16:11 ` Leif Lindholm
2017-03-01 16:31 ` [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms Ard Biesheuvel
4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-01 16:31 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
Now that we have the prerequisite functionality available in ArmMmuLib,
wire it up into ArmSetMemoryRegionNoExec, ArmClearMemoryRegionNoExec,
ArmSetMemoryRegionReadOnly and ArmClearMemoryRegionReadOnly. This is
used by the non-executable stack feature that is configured by DxeIpl.
NOTE: The current implementation will not combine RO and XP attributes,
i.e., setting/clearing a region no-exec will unconditionally
clear the read-only attribute, and vice versa. Currently, we
only use ArmSetMemoryRegionNoExec(), so for now, we should be
able to live with this.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 1112660b434e..55601328d93e 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -761,40 +761,40 @@ ArmSetMemoryAttributes (
return Status;
}
-RETURN_STATUS
+EFI_STATUS
ArmSetMemoryRegionNoExec (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
)
{
- return RETURN_UNSUPPORTED;
+ return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);
}
-RETURN_STATUS
+EFI_STATUS
ArmClearMemoryRegionNoExec (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
)
{
- return RETURN_UNSUPPORTED;
+ return ArmSetMemoryAttributes (BaseAddress, Length, 0);
}
-RETURN_STATUS
+EFI_STATUS
ArmSetMemoryRegionReadOnly (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
)
{
- return RETURN_UNSUPPORTED;
+ return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);
}
-RETURN_STATUS
+EFI_STATUS
ArmClearMemoryRegionReadOnly (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length
)
{
- return RETURN_UNSUPPORTED;
+ return ArmSetMemoryAttributes (BaseAddress, Length, 0);
}
RETURN_STATUS
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 4/5] ArmPkg/ArmMmuLib ARM: implement memory permission control routines
2017-03-01 16:31 ` [PATCH 4/5] ArmPkg/ArmMmuLib ARM: implement memory permission control routines Ard Biesheuvel
@ 2017-03-06 16:11 ` Leif Lindholm
0 siblings, 0 replies; 14+ messages in thread
From: Leif Lindholm @ 2017-03-06 16:11 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek
On Wed, Mar 01, 2017 at 04:31:42PM +0000, Ard Biesheuvel wrote:
> Now that we have the prerequisite functionality available in ArmMmuLib,
> wire it up into ArmSetMemoryRegionNoExec, ArmClearMemoryRegionNoExec,
> ArmSetMemoryRegionReadOnly and ArmClearMemoryRegionReadOnly. This is
> used by the non-executable stack feature that is configured by DxeIpl.
>
> NOTE: The current implementation will not combine RO and XP attributes,
> i.e., setting/clearing a region no-exec will unconditionally
> clear the read-only attribute, and vice versa. Currently, we
> only use ArmSetMemoryRegionNoExec(), so for now, we should be
> able to live with this.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index 1112660b434e..55601328d93e 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -761,40 +761,40 @@ ArmSetMemoryAttributes (
> return Status;
> }
>
> -RETURN_STATUS
> +EFI_STATUS
Could these RETURN_->EFI_ fixes be folded into 1/5 instead (if you've
not already pushed it by the time you get here)?
> ArmSetMemoryRegionNoExec (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> )
> {
> - return RETURN_UNSUPPORTED;
> + return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmClearMemoryRegionNoExec (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> )
> {
> - return RETURN_UNSUPPORTED;
> + return ArmSetMemoryAttributes (BaseAddress, Length, 0);
I'd be slightly happier if there was a #define for that 0, throughout.
Alternatively, replace with a macro called ArmClearMemoryAttributes().
/
Leif
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmSetMemoryRegionReadOnly (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> )
> {
> - return RETURN_UNSUPPORTED;
> + return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);
> }
>
> -RETURN_STATUS
> +EFI_STATUS
> ArmClearMemoryRegionReadOnly (
> IN EFI_PHYSICAL_ADDRESS BaseAddress,
> IN UINT64 Length
> )
> {
> - return RETURN_UNSUPPORTED;
> + return ArmSetMemoryAttributes (BaseAddress, Length, 0);
> }
>
> RETURN_STATUS
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms
2017-03-01 16:31 [PATCH 0/5] ArmPkg, ArmVirtPkg ARM: enable non-executable stack Ard Biesheuvel
` (3 preceding siblings ...)
2017-03-01 16:31 ` [PATCH 4/5] ArmPkg/ArmMmuLib ARM: implement memory permission control routines Ard Biesheuvel
@ 2017-03-01 16:31 ` Ard Biesheuvel
2017-03-01 19:10 ` Laszlo Ersek
4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-01 16:31 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
Now that ARM has grown support for managing memory permissions in
ArmMmuLib, we can enable the non-executable DXE stack for all virt
platforms.
Note that this is not [entirely] redundant: the non-executable stack
is configured before DxeCore is invoked. The image and memory protection
features configured during DXE only take affect when the CPU arch
protocol implementation is registered.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmVirtPkg/ArmVirt.dsc.inc | 5 +++++
ArmVirtPkg/ArmVirtQemu.dsc | 2 --
ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 --
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index acfb71d3ff6c..e2d3dcce7945 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -386,6 +386,11 @@ [PcdsFixedAtBuild.common]
#
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
+ #
+ # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
+ #
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
+
[PcdsFixedAtBuild.ARM]
gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 615e1fca4877..477dfdcfc764 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -152,8 +152,6 @@ [PcdsFixedAtBuild.common]
gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
[PcdsFixedAtBuild.AARCH64]
- gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
# KVM limits it IPA space to 40 bits (1 TB), so there is no need to
# support anything bigger, even if the host hardware does
gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index e4902690123c..fd39c2802a85 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -163,8 +163,6 @@ [PcdsFixedAtBuild.AARCH64]
#
gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
- gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
# KVM limits it IPA space to 40 bits (1 TB), so there is no need to
# support anything bigger, even if the host hardware does
gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms
2017-03-01 16:31 ` [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms Ard Biesheuvel
@ 2017-03-01 19:10 ` Laszlo Ersek
2017-03-01 19:10 ` Ard Biesheuvel
0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2017-03-01 19:10 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
On 03/01/17 17:31, Ard Biesheuvel wrote:
> Now that ARM has grown support for managing memory permissions in
> ArmMmuLib, we can enable the non-executable DXE stack for all virt
> platforms.
>
> Note that this is not [entirely] redundant: the non-executable stack
> is configured before DxeCore is invoked. The image and memory protection
> features configured during DXE only take affect when the CPU arch
> protocol implementation is registered.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmVirtPkg/ArmVirt.dsc.inc | 5 +++++
> ArmVirtPkg/ArmVirtQemu.dsc | 2 --
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 --
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index acfb71d3ff6c..e2d3dcce7945 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -386,6 +386,11 @@ [PcdsFixedAtBuild.common]
> #
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>
> + #
> + # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
> + #
> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> +
> [PcdsFixedAtBuild.ARM]
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 615e1fca4877..477dfdcfc764 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -152,8 +152,6 @@ [PcdsFixedAtBuild.common]
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>
> [PcdsFixedAtBuild.AARCH64]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> -
> # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
> # support anything bigger, even if the host hardware does
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index e4902690123c..fd39c2802a85 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -163,8 +163,6 @@ [PcdsFixedAtBuild.AARCH64]
> #
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>
> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> -
> # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
> # support anything bigger, even if the host hardware does
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>
This doesn't just extend PcdSetNxForStack from AARCH64 from ARM, but
also from QEMU to Xen. Is that your intent? If so,
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms
2017-03-01 19:10 ` Laszlo Ersek
@ 2017-03-01 19:10 ` Ard Biesheuvel
0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2017-03-01 19:10 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Leif Lindholm
On 1 March 2017 at 19:10, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/01/17 17:31, Ard Biesheuvel wrote:
>> Now that ARM has grown support for managing memory permissions in
>> ArmMmuLib, we can enable the non-executable DXE stack for all virt
>> platforms.
>>
>> Note that this is not [entirely] redundant: the non-executable stack
>> is configured before DxeCore is invoked. The image and memory protection
>> features configured during DXE only take affect when the CPU arch
>> protocol implementation is registered.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> ArmVirtPkg/ArmVirt.dsc.inc | 5 +++++
>> ArmVirtPkg/ArmVirtQemu.dsc | 2 --
>> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 --
>> 3 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index acfb71d3ff6c..e2d3dcce7945 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -386,6 +386,11 @@ [PcdsFixedAtBuild.common]
>> #
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>>
>> + #
>> + # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
>> + #
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> +
>> [PcdsFixedAtBuild.ARM]
>> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>>
>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>> index 615e1fca4877..477dfdcfc764 100644
>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>> @@ -152,8 +152,6 @@ [PcdsFixedAtBuild.common]
>> gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>>
>> [PcdsFixedAtBuild.AARCH64]
>> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> -
>> # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
>> # support anything bigger, even if the host hardware does
>> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> index e4902690123c..fd39c2802a85 100644
>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> @@ -163,8 +163,6 @@ [PcdsFixedAtBuild.AARCH64]
>> #
>> gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>>
>> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> -
>> # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
>> # support anything bigger, even if the host hardware does
>> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>>
>
> This doesn't just extend PcdSetNxForStack from AARCH64 from ARM, but
> also from QEMU to Xen. Is that your intent? If so,
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
Yes, it is, but I will mention that in the commit log
Thanks,
Ard.
^ permalink raw reply [flat|nested] 14+ messages in thread