| Message ID | 1479661970-10517-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 35718840efe3a29c981b5b0f4d2f617f9a1f2c2e |
| Headers | show |
On Sun, Nov 20, 2016 at 05:12:50PM +0000, Ard Biesheuvel wrote: > Translation table walks are always cache coherent on ARMv8-A, so cache > maintenance on page tables is never needed. Since there is a risk of > loss of coherency when using mismatched attributes, and given that memory > is mapped cacheable except for extraordinary cases (such as non-coherent > DMA), restrict the page table walker to performing cacheable accesses to > the translation tables. > > For DEBUG builds, retain some of the logic so that we can double check > that the memory holding the root translation table is indeed located in > memory that is mapped cacheable. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Thanks, pushed as 35718840efe3a29c981b5b0f4d2f617f9a1f2c2e. > --- > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 49 ++++++++++---------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > index 1fb3bbec6347..c78297084207 100644 > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > @@ -627,6 +627,19 @@ ArmConfigureMmu ( > return RETURN_UNSUPPORTED; > } > > + // > + // Translation table walks are always cache coherent on ARMv8-A, so cache > + // maintenance on page tables is never needed. Since there is a risk of > + // loss of coherency when using mismatched attributes, and given that memory > + // is mapped cacheable except for extraordinary cases (such as non-coherent > + // DMA), have the page table walker perform cached accesses as well, and > + // assert below that that matches the attributes we use for CPU accesses to > + // the region. > + // > + TCR |= TCR_SH_INNER_SHAREABLE | > + TCR_RGN_OUTER_WRITE_BACK_ALLOC | > + TCR_RGN_INNER_WRITE_BACK_ALLOC; > + > // Set TCR > ArmSetTCR (TCR); > > @@ -672,11 +685,15 @@ ArmConfigureMmu ( > > TranslationTableAttribute = TT_ATTR_INDX_INVALID; > while (MemoryTable->Length != 0) { > - // Find the memory attribute for the Translation Table > - if (((UINTN)TranslationTable >= MemoryTable->PhysicalBase) && > - ((UINTN)TranslationTable <= MemoryTable->PhysicalBase - 1 + MemoryTable->Length)) { > - TranslationTableAttribute = MemoryTable->Attributes; > - } > + > + DEBUG_CODE_BEGIN (); > + // Find the memory attribute for the Translation Table > + if ((UINTN)TranslationTable >= MemoryTable->PhysicalBase && > + (UINTN)TranslationTable + RootTableEntrySize <= MemoryTable->PhysicalBase + > + MemoryTable->Length) { > + TranslationTableAttribute = MemoryTable->Attributes; > + } > + DEBUG_CODE_END (); > > Status = FillTranslationTable (TranslationTable, MemoryTable); > if (RETURN_ERROR (Status)) { > @@ -685,26 +702,8 @@ ArmConfigureMmu ( > MemoryTable++; > } > > - // Translate the Memory Attributes into Translation Table Register Attributes > - if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED) || > - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED)) { > - TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_NON_CACHEABLE | TCR_RGN_INNER_NON_CACHEABLE; > - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) || > - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK)) { > - TCR |= TCR_SH_INNER_SHAREABLE | TCR_RGN_OUTER_WRITE_BACK_ALLOC | TCR_RGN_INNER_WRITE_BACK_ALLOC; > - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH) || > - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH)) { > - TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_WRITE_THROUGH | TCR_RGN_INNER_WRITE_THROUGH; > - } else { > - // If we failed to find a mapping that contains the root translation table then it probably means the translation table > - // is not mapped in the given memory map. > - ASSERT (0); > - Status = RETURN_UNSUPPORTED; > - goto FREE_TRANSLATION_TABLE; > - } > - > - // Set again TCR after getting the Translation Table attributes > - ArmSetTCR (TCR); > + ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || > + TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK); > > ArmSetMAIR (MAIR_ATTR(TT_ATTR_INDX_DEVICE_MEMORY, MAIR_ATTR_DEVICE_MEMORY) | // mapped to EFI_MEMORY_UC > MAIR_ATTR(TT_ATTR_INDX_MEMORY_NON_CACHEABLE, MAIR_ATTR_NORMAL_MEMORY_NON_CACHEABLE) | // mapped to EFI_MEMORY_WC > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c index 1fb3bbec6347..c78297084207 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c @@ -627,6 +627,19 @@ ArmConfigureMmu ( return RETURN_UNSUPPORTED; } + // + // Translation table walks are always cache coherent on ARMv8-A, so cache + // maintenance on page tables is never needed. Since there is a risk of + // loss of coherency when using mismatched attributes, and given that memory + // is mapped cacheable except for extraordinary cases (such as non-coherent + // DMA), have the page table walker perform cached accesses as well, and + // assert below that that matches the attributes we use for CPU accesses to + // the region. + // + TCR |= TCR_SH_INNER_SHAREABLE | + TCR_RGN_OUTER_WRITE_BACK_ALLOC | + TCR_RGN_INNER_WRITE_BACK_ALLOC; + // Set TCR ArmSetTCR (TCR); @@ -672,11 +685,15 @@ ArmConfigureMmu ( TranslationTableAttribute = TT_ATTR_INDX_INVALID; while (MemoryTable->Length != 0) { - // Find the memory attribute for the Translation Table - if (((UINTN)TranslationTable >= MemoryTable->PhysicalBase) && - ((UINTN)TranslationTable <= MemoryTable->PhysicalBase - 1 + MemoryTable->Length)) { - TranslationTableAttribute = MemoryTable->Attributes; - } + + DEBUG_CODE_BEGIN (); + // Find the memory attribute for the Translation Table + if ((UINTN)TranslationTable >= MemoryTable->PhysicalBase && + (UINTN)TranslationTable + RootTableEntrySize <= MemoryTable->PhysicalBase + + MemoryTable->Length) { + TranslationTableAttribute = MemoryTable->Attributes; + } + DEBUG_CODE_END (); Status = FillTranslationTable (TranslationTable, MemoryTable); if (RETURN_ERROR (Status)) { @@ -685,26 +702,8 @@ ArmConfigureMmu ( MemoryTable++; } - // Translate the Memory Attributes into Translation Table Register Attributes - if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED) || - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED)) { - TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_NON_CACHEABLE | TCR_RGN_INNER_NON_CACHEABLE; - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) || - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK)) { - TCR |= TCR_SH_INNER_SHAREABLE | TCR_RGN_OUTER_WRITE_BACK_ALLOC | TCR_RGN_INNER_WRITE_BACK_ALLOC; - } else if ((TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH) || - (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH)) { - TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_WRITE_THROUGH | TCR_RGN_INNER_WRITE_THROUGH; - } else { - // If we failed to find a mapping that contains the root translation table then it probably means the translation table - // is not mapped in the given memory map. - ASSERT (0); - Status = RETURN_UNSUPPORTED; - goto FREE_TRANSLATION_TABLE; - } - - // Set again TCR after getting the Translation Table attributes - ArmSetTCR (TCR); + ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || + TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK); ArmSetMAIR (MAIR_ATTR(TT_ATTR_INDX_DEVICE_MEMORY, MAIR_ATTR_DEVICE_MEMORY) | // mapped to EFI_MEMORY_UC MAIR_ATTR(TT_ATTR_INDX_MEMORY_NON_CACHEABLE, MAIR_ATTR_NORMAL_MEMORY_NON_CACHEABLE) | // mapped to EFI_MEMORY_WC
Translation table walks are always cache coherent on ARMv8-A, so cache maintenance on page tables is never needed. Since there is a risk of loss of coherency when using mismatched attributes, and given that memory is mapped cacheable except for extraordinary cases (such as non-coherent DMA), restrict the page table walker to performing cacheable accesses to the translation tables. For DEBUG builds, retain some of the logic so that we can double check that the memory holding the root translation table is indeed located in memory that is mapped cacheable. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 49 ++++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel