Re: [edk2] [PATCH v2] ArmPkg/ArmLib: don't invalidate entire I-cache on range operation

2016-05-12 Thread Leif Lindholm 
On 11 May 2016 at 11:13, Ard Biesheuvel  wrote:
> Instead of cleaning the data cache to the PoU by virtual address and
> subsequently invalidating the entire I-cache, invalidate only the
> range that we just cleaned. This way, we don't invalidate other
> cachelines unnecessarily.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
> v2:
> - use correct stride for Icache maintenance by VA
> - issue an ISB after completing the Icache maintenance
>
>  ArmPkg/Include/Library/ArmLib.h| 10 +++--
>  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 23 
> +++-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S |  5 +
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S |  5 +
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   |  6 +
>  5 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 1689f0072db6..4608b0cc 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -183,13 +183,19 @@ ArmInvalidateDataCacheEntryByMVA (
>
>  VOID
>  EFIAPI
> -ArmCleanDataCacheEntryToPoUByMVA(
> +ArmCleanDataCacheEntryToPoUByMVA (
>IN  UINTN   Address
>);
>
>  VOID
>  EFIAPI
> -ArmCleanDataCacheEntryByMVA(
> +ArmInvalidateInstructionCacheEntryToPoUByMVA (
> +  IN  UINTN   Address
> +  );
> +
> +VOID
> +EFIAPI
> +ArmCleanDataCacheEntryByMVA (
>  IN  UINTN   Address
>  );
>
> diff --git a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> index 1045f9068f4d..f7667b8552d2 100644
> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> @@ -17,11 +17,13 @@
>  #include 
>  #include 
>
> +STATIC
>  VOID
>  CacheRangeOperation (
>IN  VOID*Start,
>IN  UINTN   Length,
> -  IN  LINE_OPERATION  LineOperation
> +  IN  LINE_OPERATION  LineOperation,
> +  IN  UINTN   LineLength
>)
>  {
>UINTN ArmCacheLineLength = ArmDataCacheLineLength();
> @@ -64,8 +66,14 @@ InvalidateInstructionCacheRange (
>IN  UINTN Length
>)
>  {
> -  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
> -  ArmInvalidateInstructionCache ();
> +  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA,
> +ArmDataCacheLineLength ());
> +  CacheRangeOperation (Address, Length,
> +ArmInvalidateInstructionCacheEntryToPoUByMVA,
> +ArmInstructionCacheLineLength ());
> +
> +  ArmInstructionSynchronizationBarrier ();
> +
>return Address;
>  }
>
> @@ -85,7 +93,8 @@ WriteBackInvalidateDataCacheRange (
>IN  UINTN Length
>)
>  {
> -  CacheRangeOperation(Address, Length, 
> ArmCleanInvalidateDataCacheEntryByMVA);
> +  CacheRangeOperation(Address, Length, ArmCleanInvalidateDataCacheEntryByMVA,
> +ArmDataCacheLineLength ());
>return Address;
>  }
>
> @@ -105,7 +114,8 @@ WriteBackDataCacheRange (
>IN  UINTN Length
>)
>  {
> -  CacheRangeOperation(Address, Length, ArmCleanDataCacheEntryByMVA);
> +  CacheRangeOperation(Address, Length, ArmCleanDataCacheEntryByMVA,
> +ArmDataCacheLineLength ());
>return Address;
>  }
>
> @@ -116,6 +126,7 @@ InvalidateDataCacheRange (
>IN  UINTN Length
>)
>  {
> -  CacheRangeOperation(Address, Length, ArmInvalidateDataCacheEntryByMVA);
> +  CacheRangeOperation(Address, Length, ArmInvalidateDataCacheEntryByMVA,
> +ArmDataCacheLineLength ());
>return Address;
>  }
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> index 43f7a795acec..9441f47e30ba 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> @@ -23,6 +23,7 @@ GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> +GCC_ASM_EXPORT (ArmInvalidateInstructionCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> @@ -80,6 +81,10 @@ ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
>dc  cvau, x0// Clean single data cache line to PoU
>ret
>
> +ASM_PFX(ArmInvalidateInstructionCacheEntryToPoUByMVA):
> +  ic  ivau, x0// Invalidate single instruction cache line to PoU
> +  ret
> +
>
>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>dc  civac, x0   // Clean and

[edk2] [PATCH v2] ArmPkg/ArmLib: don't invalidate entire I-cache on range operation

2016-05-11 Thread Ard Biesheuvel 
Instead of cleaning the data cache to the PoU by virtual address and
subsequently invalidating the entire I-cache, invalidate only the
range that we just cleaned. This way, we don't invalidate other
cachelines unnecessarily.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
v2:
- use correct stride for Icache maintenance by VA
- issue an ISB after completing the Icache maintenance

 ArmPkg/Include/Library/ArmLib.h| 10 +++--
 ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 23 
+++-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S |  5 +
 ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S |  5 +
 ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   |  6 +
 5 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 1689f0072db6..4608b0cc 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -183,13 +183,19 @@ ArmInvalidateDataCacheEntryByMVA (
 
 VOID
 EFIAPI
-ArmCleanDataCacheEntryToPoUByMVA(
+ArmCleanDataCacheEntryToPoUByMVA (
   IN  UINTN   Address
   );
 
 VOID
 EFIAPI
-ArmCleanDataCacheEntryByMVA(
+ArmInvalidateInstructionCacheEntryToPoUByMVA (
+  IN  UINTN   Address
+  );
+
+VOID
+EFIAPI
+ArmCleanDataCacheEntryByMVA (
 IN  UINTN   Address
 );
 
diff --git a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
index 1045f9068f4d..f7667b8552d2 100644
--- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
+++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
@@ -17,11 +17,13 @@
 #include 
 #include 
 
+STATIC
 VOID
 CacheRangeOperation (
   IN  VOID*Start,
   IN  UINTN   Length,
-  IN  LINE_OPERATION  LineOperation
+  IN  LINE_OPERATION  LineOperation,
+  IN  UINTN   LineLength
   )
 {
   UINTN ArmCacheLineLength = ArmDataCacheLineLength();
@@ -64,8 +66,14 @@ InvalidateInstructionCacheRange (
   IN  UINTN Length
   )
 {
-  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
-  ArmInvalidateInstructionCache ();
+  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA,
+ArmDataCacheLineLength ());
+  CacheRangeOperation (Address, Length,
+ArmInvalidateInstructionCacheEntryToPoUByMVA,
+ArmInstructionCacheLineLength ());
+
+  ArmInstructionSynchronizationBarrier ();
+
   return Address;
 }
 
@@ -85,7 +93,8 @@ WriteBackInvalidateDataCacheRange (
   IN  UINTN Length
   )
 {
-  CacheRangeOperation(Address, Length, ArmCleanInvalidateDataCacheEntryByMVA);
+  CacheRangeOperation(Address, Length, ArmCleanInvalidateDataCacheEntryByMVA,
+ArmDataCacheLineLength ());
   return Address;
 }
 
@@ -105,7 +114,8 @@ WriteBackDataCacheRange (
   IN  UINTN Length
   )
 {
-  CacheRangeOperation(Address, Length, ArmCleanDataCacheEntryByMVA);
+  CacheRangeOperation(Address, Length, ArmCleanDataCacheEntryByMVA,
+ArmDataCacheLineLength ());
   return Address;
 }
 
@@ -116,6 +126,7 @@ InvalidateDataCacheRange (
   IN  UINTN Length
   )
 {
-  CacheRangeOperation(Address, Length, ArmInvalidateDataCacheEntryByMVA);
+  CacheRangeOperation(Address, Length, ArmInvalidateDataCacheEntryByMVA,
+ArmDataCacheLineLength ());
   return Address;
 }
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index 43f7a795acec..9441f47e30ba 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -23,6 +23,7 @@ GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
 GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
 GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
 GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
+GCC_ASM_EXPORT (ArmInvalidateInstructionCacheEntryToPoUByMVA)
 GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
 GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
 GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
@@ -80,6 +81,10 @@ ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
   dc  cvau, x0// Clean single data cache line to PoU
   ret
 
+ASM_PFX(ArmInvalidateInstructionCacheEntryToPoUByMVA):
+  ic  ivau, x0// Invalidate single instruction cache line to PoU
+  ret
+
 
 ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
   dc  civac, x0   // Clean and invalidate single data cache line
diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S 
b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
index 50c760f335de..c765032c9e4d 100644
--- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
+++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
@@ -18,6 +18,7 @@
 
 GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
 GCC_ASM_EXPORT