Alex reports that the cache invalidation performed by
ArmVirtMemoryInitPeiLib takes a non-negligible amount of time at boot.
This cache invalidation used to be necessary to avoid inconsistencies
between the CPU's cached and uncached views of the permanent PEI memory
region, given that the PEI phase is where the MMU gets enabled.

The only allocations done from permanent PEI memory with the MMU off are
pages used for page tables, and since commit 748fea6279ef
("ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating
them"), each of those is invalidated in the caches explicitly, for
reasons described in the patch's commit log. All other allocations done
in PEI are either from temporary PEI memory, which includes the stack,
or from permanent PEI memory but after the MMU has been enabled.

This means that the cache invalidation in ArmVirtMemoryInitPeiLib is no
longer necessary, and can simply be dropped.

Cc: Alexander Graf <agraf@csgraf.de>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 .../ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c    | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
index 022e13e762b6..98d90ad4200d 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
@@ -91,15 +91,6 @@ MemoryPeim (
       );
   }
 
-  //
-  // When running under virtualization, the PI/UEFI memory region may be
-  // clean but not invalidated in system caches or in lower level caches
-  // on other CPUs. So invalidate the region by virtual address, to ensure
-  // that the contents we put there with the caches and MMU off will still
-  // be visible after turning them on.
-  //
-  InvalidateDataCacheRange ((VOID *)(UINTN)UefiMemoryBase, UefiMemorySize);
-
   // Build Memory Allocation Hob
   InitMmu ();
 
-- 
2.30.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86203): https://edk2.groups.io/g/devel/message/86203
Mute This Topic: https://groups.io/mt/88767635/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 29.01.22 16:13, Ard Biesheuvel wrote:
> Alex reports that the cache invalidation performed by
> ArmVirtMemoryInitPeiLib takes a non-negligible amount of time at boot.
> This cache invalidation used to be necessary to avoid inconsistencies
> between the CPU's cached and uncached views of the permanent PEI memory
> region, given that the PEI phase is where the MMU gets enabled.
>
> The only allocations done from permanent PEI memory with the MMU off are
> pages used for page tables, and since commit 748fea6279ef
> ("ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating
> them"), each of those is invalidated in the caches explicitly, for
> reasons described in the patch's commit log. All other allocations done
> in PEI are either from temporary PEI memory, which includes the stack,
> or from permanent PEI memory but after the MMU has been enabled.
>
> This means that the cache invalidation in ArmVirtMemoryInitPeiLib is no
> longer necessary, and can simply be dropped.
>
> Cc: Alexander Graf <agraf@csgraf.de>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>


Reviewed-by: Alexander Graf <graf@amazon.com>

Also, feel free to add

Reported-by: Alexander Graf <graf@amazon.com>


Thanks a bunch!

Alex


> ---
>   .../ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c    | 9 ---------
>   1 file changed, 9 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> index 022e13e762b6..98d90ad4200d 100644
> --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> @@ -91,15 +91,6 @@ MemoryPeim (
>         );
>     }
>   
> -  //
> -  // When running under virtualization, the PI/UEFI memory region may be
> -  // clean but not invalidated in system caches or in lower level caches
> -  // on other CPUs. So invalidate the region by virtual address, to ensure
> -  // that the contents we put there with the caches and MMU off will still
> -  // be visible after turning them on.
> -  //
> -  InvalidateDataCacheRange ((VOID *)(UINTN)UefiMemoryBase, UefiMemorySize);
> -
>     // Build Memory Allocation Hob
>     InitMmu ();
>   



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86251): https://edk2.groups.io/g/devel/message/86251
Mute This Topic: https://groups.io/mt/88767635/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Mon, 31 Jan 2022 at 14:21, Alexander Graf <graf@amazon.com> wrote:
>
>
> On 29.01.22 16:13, Ard Biesheuvel wrote:
> > Alex reports that the cache invalidation performed by
> > ArmVirtMemoryInitPeiLib takes a non-negligible amount of time at boot.
> > This cache invalidation used to be necessary to avoid inconsistencies
> > between the CPU's cached and uncached views of the permanent PEI memory
> > region, given that the PEI phase is where the MMU gets enabled.
> >
> > The only allocations done from permanent PEI memory with the MMU off are
> > pages used for page tables, and since commit 748fea6279ef
> > ("ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating
> > them"), each of those is invalidated in the caches explicitly, for
> > reasons described in the patch's commit log. All other allocations done
> > in PEI are either from temporary PEI memory, which includes the stack,
> > or from permanent PEI memory but after the MMU has been enabled.
> >
> > This means that the cache invalidation in ArmVirtMemoryInitPeiLib is no
> > longer necessary, and can simply be dropped.
> >
> > Cc: Alexander Graf <agraf@csgraf.de>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
>
> Reviewed-by: Alexander Graf <graf@amazon.com>
>
> Also, feel free to add
>
> Reported-by: Alexander Graf <graf@amazon.com>
>

Merged as #2547

Thanks,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87026): https://edk2.groups.io/g/devel/message/87026
Mute This Topic: https://groups.io/mt/88767635/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Sat, Jan 29, 2022 at 16:13:46 +0100, Ard Biesheuvel wrote:
> Alex reports that the cache invalidation performed by
> ArmVirtMemoryInitPeiLib takes a non-negligible amount of time at boot.
> This cache invalidation used to be necessary to avoid inconsistencies
> between the CPU's cached and uncached views of the permanent PEI memory
> region, given that the PEI phase is where the MMU gets enabled.
> 
> The only allocations done from permanent PEI memory with the MMU off are
> pages used for page tables, and since commit 748fea6279ef
> ("ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating
> them"), each of those is invalidated in the caches explicitly, for
> reasons described in the patch's commit log. All other allocations done
> in PEI are either from temporary PEI memory, which includes the stack,
> or from permanent PEI memory but after the MMU has been enabled.
> 
> This means that the cache invalidation in ArmVirtMemoryInitPeiLib is no
> longer necessary, and can simply be dropped.
> 
> Cc: Alexander Graf <agraf@csgraf.de>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Leif Lindholm <leif@nuviainc.com>

> ---
>  .../ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c    | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> index 022e13e762b6..98d90ad4200d 100644
> --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> @@ -91,15 +91,6 @@ MemoryPeim (
>        );
>    }
>  
> -  //
> -  // When running under virtualization, the PI/UEFI memory region may be
> -  // clean but not invalidated in system caches or in lower level caches
> -  // on other CPUs. So invalidate the region by virtual address, to ensure
> -  // that the contents we put there with the caches and MMU off will still
> -  // be visible after turning them on.
> -  //
> -  InvalidateDataCacheRange ((VOID *)(UINTN)UefiMemoryBase, UefiMemorySize);
> -
>    // Build Memory Allocation Hob
>    InitMmu ();
>  
> -- 
> 2.30.2
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86247): https://edk2.groups.io/g/devel/message/86247
Mute This Topic: https://groups.io/mt/88767635/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-