Instead of performing two cache invalidations for each section entry
that gets updated, perform the first invalidation, which is intended
to clean the page tables from caches on systems where cache hits are
permitted with the MMU and caches off.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 33 +++++++++++---------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 0800ef560d89..11a1e704beab 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -147,6 +147,13 @@ PopulateLevel2PageTable (
 
       BaseSectionAddress = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(*SectionEntry);
 
+      //
+      // Make sure we are not inadvertently hitting in the caches
+      // when populating the page tables
+      //
+      InvalidateDataCacheRange ((VOID *)TranslationTable,
+        TRANSLATION_TABLE_PAGE_SIZE);
+
       // Populate the new Level2 Page Table for the section
       PageEntry = (UINT32*)TranslationTable;
       for (Index = 0; Index < TRANSLATION_TABLE_PAGE_COUNT; Index++) {
@@ -166,6 +173,12 @@ PopulateLevel2PageTable (
     TranslationTable = (UINTN)AllocateAlignedPages (
                                 EFI_SIZE_TO_PAGES (TRANSLATION_TABLE_PAGE_SIZE),
                                 TRANSLATION_TABLE_PAGE_ALIGNMENT);
+    //
+    // Make sure we are not inadvertently hitting in the caches
+    // when populating the page tables
+    //
+    InvalidateDataCacheRange ((VOID *)TranslationTable,
+      TRANSLATION_TABLE_PAGE_SIZE);
     ZeroMem ((VOID *)TranslationTable, TRANSLATION_TABLE_PAGE_SIZE);
 
     *SectionEntry = (TranslationTable & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK) |
@@ -179,13 +192,6 @@ PopulateLevel2PageTable (
 
   ASSERT (FirstPageOffset + Pages <= TRANSLATION_TABLE_PAGE_COUNT);
 
-  //
-  // Invalidate once to prevent page table updates to hit in the
-  // caches inadvertently.
-  //
-  InvalidateDataCacheRange ((UINT32 *)TranslationTable + FirstPageOffset,
-    RemainLength / TT_DESCRIPTOR_PAGE_SIZE * sizeof (*PageEntry));
-
   for (Index = 0; Index < Pages; Index++) {
     *PageEntry++     =  TT_DESCRIPTOR_PAGE_BASE_ADDRESS(PhysicalBase) | PageAttributes;
     PhysicalBase += TT_DESCRIPTOR_PAGE_SIZE;
@@ -268,14 +274,6 @@ FillTranslationTable (
   SectionEntry    = TRANSLATION_TABLE_ENTRY_FOR_VIRTUAL_ADDRESS(TranslationTable, MemoryRegion->VirtualBase);
 
   while (RemainLength != 0) {
-    //
-    // Ensure that the assignment of the page table entry will not hit
-    // in the cache. Whether this could occur is IMPLEMENTATION DEFINED
-    // and thus permitted by the ARMv7 architecture.
-    //
-    ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry);
-    ArmDataSynchronizationBarrier ();
-
     if (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE == 0 &&
         RemainLength >= TT_DESCRIPTOR_SECTION_SIZE) {
       // Case: Physical address aligned on the Section Size (1MB) && the length
@@ -348,6 +346,11 @@ ArmConfigureMmu (
     *TranslationTableSize = TRANSLATION_TABLE_SECTION_SIZE;
   }
 
+  //
+  // Make sure we are not inadvertently hitting in the caches
+  // when populating the page tables
+  //
+  InvalidateDataCacheRange (TranslationTable, TRANSLATION_TABLE_SECTION_SIZE);
   ZeroMem (TranslationTable, TRANSLATION_TABLE_SECTION_SIZE);
 
   // By default, mark the translation table as belonging to a uncached region
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55492): https://edk2.groups.io/g/devel/message/55492
Mute This Topic: https://groups.io/mt/71746597/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Thu, 5 Mar 2020 at 11:00, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> Instead of performing two cache invalidations for each section entry
> that gets updated, perform the first invalidation, which is intended
> to clean the page tables from caches on systems where cache hits are
> permitted with the MMU and caches off.
>

... as the page tables are allocated.

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 33 +++++++++++---------
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index 0800ef560d89..11a1e704beab 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -147,6 +147,13 @@ PopulateLevel2PageTable (
>
>        BaseSectionAddress = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(*SectionEntry);
>
> +      //
> +      // Make sure we are not inadvertently hitting in the caches
> +      // when populating the page tables
> +      //
> +      InvalidateDataCacheRange ((VOID *)TranslationTable,
> +        TRANSLATION_TABLE_PAGE_SIZE);
> +
>        // Populate the new Level2 Page Table for the section
>        PageEntry = (UINT32*)TranslationTable;
>        for (Index = 0; Index < TRANSLATION_TABLE_PAGE_COUNT; Index++) {
> @@ -166,6 +173,12 @@ PopulateLevel2PageTable (
>      TranslationTable = (UINTN)AllocateAlignedPages (
>                                  EFI_SIZE_TO_PAGES (TRANSLATION_TABLE_PAGE_SIZE),
>                                  TRANSLATION_TABLE_PAGE_ALIGNMENT);
> +    //
> +    // Make sure we are not inadvertently hitting in the caches
> +    // when populating the page tables
> +    //
> +    InvalidateDataCacheRange ((VOID *)TranslationTable,
> +      TRANSLATION_TABLE_PAGE_SIZE);
>      ZeroMem ((VOID *)TranslationTable, TRANSLATION_TABLE_PAGE_SIZE);
>
>      *SectionEntry = (TranslationTable & TT_DESCRIPTOR_SECTION_PAGETABLE_ADDRESS_MASK) |
> @@ -179,13 +192,6 @@ PopulateLevel2PageTable (
>
>    ASSERT (FirstPageOffset + Pages <= TRANSLATION_TABLE_PAGE_COUNT);
>
> -  //
> -  // Invalidate once to prevent page table updates to hit in the
> -  // caches inadvertently.
> -  //
> -  InvalidateDataCacheRange ((UINT32 *)TranslationTable + FirstPageOffset,
> -    RemainLength / TT_DESCRIPTOR_PAGE_SIZE * sizeof (*PageEntry));
> -
>    for (Index = 0; Index < Pages; Index++) {
>      *PageEntry++     =  TT_DESCRIPTOR_PAGE_BASE_ADDRESS(PhysicalBase) | PageAttributes;
>      PhysicalBase += TT_DESCRIPTOR_PAGE_SIZE;
> @@ -268,14 +274,6 @@ FillTranslationTable (
>    SectionEntry    = TRANSLATION_TABLE_ENTRY_FOR_VIRTUAL_ADDRESS(TranslationTable, MemoryRegion->VirtualBase);
>
>    while (RemainLength != 0) {
> -    //
> -    // Ensure that the assignment of the page table entry will not hit
> -    // in the cache. Whether this could occur is IMPLEMENTATION DEFINED
> -    // and thus permitted by the ARMv7 architecture.
> -    //
> -    ArmInvalidateDataCacheEntryByMVA ((UINTN)SectionEntry);
> -    ArmDataSynchronizationBarrier ();
> -
>      if (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE == 0 &&
>          RemainLength >= TT_DESCRIPTOR_SECTION_SIZE) {
>        // Case: Physical address aligned on the Section Size (1MB) && the length
> @@ -348,6 +346,11 @@ ArmConfigureMmu (
>      *TranslationTableSize = TRANSLATION_TABLE_SECTION_SIZE;
>    }
>
> +  //
> +  // Make sure we are not inadvertently hitting in the caches
> +  // when populating the page tables
> +  //
> +  InvalidateDataCacheRange (TranslationTable, TRANSLATION_TABLE_SECTION_SIZE);
>    ZeroMem (TranslationTable, TRANSLATION_TABLE_SECTION_SIZE);
>
>    // By default, mark the translation table as belonging to a uncached region
> --
> 2.17.1
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55496): https://edk2.groups.io/g/devel/message/55496
Mute This Topic: https://groups.io/mt/71746597/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-