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 <[email protected]>
Reviewed-by: Leif Lindholm <[email protected]> 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
