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

2016-05-11 Thread Mark Rutland 
On Wed, May 11, 2016 at 12:23:58PM +0200, Ard Biesheuvel wrote:
> On 11 May 2016 at 12:22, Mark Rutland  wrote:
> > On Wed, May 11, 2016 at 12:07:51PM +0200, Ard Biesheuvel wrote:
> >> On 11 May 2016 at 11:35, Achin Gupta  wrote:
> >> >> diff --git 
> >> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
> >> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> >> index 1045f9068f4d..cc7555061428 100644
> >> >> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> >> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> >> @@ -65,7 +65,8 @@ InvalidateInstructionCacheRange (
> >> >>)
> >> >>  {
> >> >>CacheRangeOperation (Address, Length, 
> >> >> ArmCleanDataCacheEntryToPoUByMVA);
> >> >> -  ArmInvalidateInstructionCache ();
> >> >> +  CacheRangeOperation (Address, Length,
> >> >> +ArmInvalidateInstructionCacheEntryToPoUByMVA);
> >> >
> >> > Afaics, CacheRangeOperation() uses the data cache line length by 
> >> > default. This
> >> > will match the I$ line length in most cases. But, it would be better to 
> >> > use the
> >> > ArmInstructionCacheLineLength() function instead. I suggest that we add 
> >> > a cache
> >> > line length parameter to CacheRangeOperation() to allow the distinction.
> >> >
> >>
> >> Good point.
> >>
> >> > Also, from my reading of the ARMv8 ARM B2.6.5 & E2.6.5, we need an ISB 
> >> > at the
> >> > end of all the operations.
> >> >
> >>
> >> I would assume that a single call to
> >> ArmInstructionSynchronizationBarrier() at the end of
> >> InvalidateInstructionCacheRange () should suffice?
> >
> > Almost. You also need DSBs to complete the maintenance ops. e.g. a
> > sequence:
> >
> > d-cache maintenance ops
> > DSB
> > i-cache maintenance ops
> > DSB
> > ISB
> >
> 
> Yes, but the DSB is already performed by CacheRangeOperation (). So
> adding the ISB in InvalidateInstructionCacheRange () results in
> exactly the above.

Ah, sorry, I managed to miss that when scanning the source code.

I guess I'm just saying "yes" then. :)

THanks,
Mark.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2016-05-11 Thread Ard Biesheuvel 
On 11 May 2016 at 12:22, Mark Rutland  wrote:
> On Wed, May 11, 2016 at 12:07:51PM +0200, Ard Biesheuvel wrote:
>> On 11 May 2016 at 11:35, Achin Gupta  wrote:
>> >> diff --git 
>> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
>> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
>> >> index 1045f9068f4d..cc7555061428 100644
>> >> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
>> >> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
>> >> @@ -65,7 +65,8 @@ InvalidateInstructionCacheRange (
>> >>)
>> >>  {
>> >>CacheRangeOperation (Address, Length, 
>> >> ArmCleanDataCacheEntryToPoUByMVA);
>> >> -  ArmInvalidateInstructionCache ();
>> >> +  CacheRangeOperation (Address, Length,
>> >> +ArmInvalidateInstructionCacheEntryToPoUByMVA);
>> >
>> > Afaics, CacheRangeOperation() uses the data cache line length by default. 
>> > This
>> > will match the I$ line length in most cases. But, it would be better to 
>> > use the
>> > ArmInstructionCacheLineLength() function instead. I suggest that we add a 
>> > cache
>> > line length parameter to CacheRangeOperation() to allow the distinction.
>> >
>>
>> Good point.
>>
>> > Also, from my reading of the ARMv8 ARM B2.6.5 & E2.6.5, we need an ISB at 
>> > the
>> > end of all the operations.
>> >
>>
>> I would assume that a single call to
>> ArmInstructionSynchronizationBarrier() at the end of
>> InvalidateInstructionCacheRange () should suffice?
>
> Almost. You also need DSBs to complete the maintenance ops. e.g. a
> sequence:
>
> d-cache maintenance ops
> DSB
> i-cache maintenance ops
> DSB
> ISB
>

Yes, but the DSB is already performed by CacheRangeOperation (). So
adding the ISB in InvalidateInstructionCacheRange () results in
exactly the above.

Thanks,
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2016-05-11 Thread Mark Rutland 
On Wed, May 11, 2016 at 12:07:51PM +0200, Ard Biesheuvel wrote:
> On 11 May 2016 at 11:35, Achin Gupta  wrote:
> >> diff --git 
> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> index 1045f9068f4d..cc7555061428 100644
> >> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> @@ -65,7 +65,8 @@ InvalidateInstructionCacheRange (
> >>)
> >>  {
> >>CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
> >> -  ArmInvalidateInstructionCache ();
> >> +  CacheRangeOperation (Address, Length,
> >> +ArmInvalidateInstructionCacheEntryToPoUByMVA);
> >
> > Afaics, CacheRangeOperation() uses the data cache line length by default. 
> > This
> > will match the I$ line length in most cases. But, it would be better to use 
> > the
> > ArmInstructionCacheLineLength() function instead. I suggest that we add a 
> > cache
> > line length parameter to CacheRangeOperation() to allow the distinction.
> >
> 
> Good point.
> 
> > Also, from my reading of the ARMv8 ARM B2.6.5 & E2.6.5, we need an ISB at 
> > the
> > end of all the operations.
> >
> 
> I would assume that a single call to
> ArmInstructionSynchronizationBarrier() at the end of
> InvalidateInstructionCacheRange () should suffice?

Almost. You also need DSBs to complete the maintenance ops. e.g. a
sequence:

d-cache maintenance ops
DSB
i-cache maintenance ops
DSB
ISB

Thanks,
Mark.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2016-05-11 Thread Achin Gupta 
On Wed, May 11, 2016 at 12:07:51PM +0200, Ard Biesheuvel wrote:
> On 11 May 2016 at 11:35, Achin Gupta  wrote:
> > Hi Ard,
> >
> > Some comments inline!
> >
> > On Wed, May 11, 2016 at 10:41:57AM +0200, 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 
> >> ---
> >>  ArmPkg/Include/Library/ArmLib.h| 10 
> >> --
> >>  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c |  3 ++-
> >>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S |  5 +
> >>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S |  5 +
> >>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   |  6 ++
> >>  5 files changed, 26 insertions(+), 3 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..cc7555061428 100644
> >> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >> @@ -65,7 +65,8 @@ InvalidateInstructionCacheRange (
> >>)
> >>  {
> >>CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
> >> -  ArmInvalidateInstructionCache ();
> >> +  CacheRangeOperation (Address, Length,
> >> +ArmInvalidateInstructionCacheEntryToPoUByMVA);
> >
> > Afaics, CacheRangeOperation() uses the data cache line length by default. 
> > This
> > will match the I$ line length in most cases. But, it would be better to use 
> > the
> > ArmInstructionCacheLineLength() function instead. I suggest that we add a 
> > cache
> > line length parameter to CacheRangeOperation() to allow the distinction.
> >
>
> Good point.
>
> > Also, from my reading of the ARMv8 ARM B2.6.5 & E2.6.5, we need an ISB at 
> > the
> > end of all the operations.
> >
>
> I would assume that a single call to
> ArmInstructionSynchronizationBarrier() at the end of
> InvalidateInstructionCacheRange () should suffice?

Agree. This is what I am doing locally:

@@ -64,8 +64,9 @@ InvalidateInstructionCacheRange (
   IN  UINTN Length
   )
 {
-  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
-  ArmInvalidateInstructionCache ();
+  CacheRangeOperation (Address, Length, ArmDataCacheLineLength(), 
ArmCleanDataCacheEntryToPoUByMVA);
+  CacheRangeOperation (Address, Length, ArmInstructionCacheLineLength(), 
ArmInvalidateInstructionCacheEntryToPoUByMVA);
+  ArmInstructionSynchronizationBarrier();
   return Address;
 }

>
> >>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

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

2016-05-11 Thread Ard Biesheuvel 
On 11 May 2016 at 11:35, Achin Gupta  wrote:
> Hi Ard,
>
> Some comments inline!
>
> On Wed, May 11, 2016 at 10:41:57AM +0200, 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 
>> ---
>>  ArmPkg/Include/Library/ArmLib.h| 10 
>> --
>>  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c |  3 ++-
>>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S |  5 +
>>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S |  5 +
>>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   |  6 ++
>>  5 files changed, 26 insertions(+), 3 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..cc7555061428 100644
>> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
>> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
>> @@ -65,7 +65,8 @@ InvalidateInstructionCacheRange (
>>)
>>  {
>>CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
>> -  ArmInvalidateInstructionCache ();
>> +  CacheRangeOperation (Address, Length,
>> +ArmInvalidateInstructionCacheEntryToPoUByMVA);
>
> Afaics, CacheRangeOperation() uses the data cache line length by default. This
> will match the I$ line length in most cases. But, it would be better to use 
> the
> ArmInstructionCacheLineLength() function instead. I suggest that we add a 
> cache
> line length parameter to CacheRangeOperation() to allow the distinction.
>

Good point.

> Also, from my reading of the ARMv8 ARM B2.6.5 & E2.6.5, we need an ISB at the
> end of all the operations.
>

I would assume that a single call to
ArmInstructionSynchronizationBarrier() at the end of
InvalidateInstructionCacheRange () should suffice?

>>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 (ArmInvalidateDataCacheEntryByMVA)
>> +GCC_ASM_EXPORT (ArmInvalidateInstructionCacheEntryToPoUByMVA)
>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>> @@ -74,6 +75,10 @@ ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
>>mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
>>bx  lr
>>
>> +ASM_PFX(ArmInvalidateInstructionCacheEntryToPoUByMVA):
>> +  mcr p15, 0, r0, c7, c5, 1  @Invalidate single instruction cache line 
>> to PoU
>> +  mcr p15, 0, r0, c7, c5, 7

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

2016-05-11 Thread Achin Gupta 
Hi Ard,

Some comments inline!

On Wed, May 11, 2016 at 10:41:57AM +0200, 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 
> ---
>  ArmPkg/Include/Library/ArmLib.h| 10 
> --
>  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c |  3 ++-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S |  5 +
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S |  5 +
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   |  6 ++
>  5 files changed, 26 insertions(+), 3 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..cc7555061428 100644
> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> @@ -65,7 +65,8 @@ InvalidateInstructionCacheRange (
>)
>  {
>CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
> -  ArmInvalidateInstructionCache ();
> +  CacheRangeOperation (Address, Length,
> +ArmInvalidateInstructionCacheEntryToPoUByMVA);

Afaics, CacheRangeOperation() uses the data cache line length by default. This
will match the I$ line length in most cases. But, it would be better to use the
ArmInstructionCacheLineLength() function instead. I suggest that we add a cache
line length parameter to CacheRangeOperation() to allow the distinction.

Also, from my reading of the ARMv8 ARM B2.6.5 & E2.6.5, we need an ISB at the
end of all the operations.

>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 (ArmInvalidateDataCacheEntryByMVA)
> +GCC_ASM_EXPORT (ArmInvalidateInstructionCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> @@ -74,6 +75,10 @@ ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
>mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
>bx  lr
>
> +ASM_PFX(ArmInvalidateInstructionCacheEntryToPoUByMVA):
> +  mcr p15, 0, r0, c7, c5, 1  @Invalidate single instruction cache line 
> to PoU
> +  mcr p15, 0, r0, c7, c5, 7  @Invalidate branch predictor

Is it reasonable to assume that for every Icache invalidation by MVA, the BP
will have to be invalidated for that MVA as well? I guess so!

cheers,
Achin

> +  bx  lr
>
>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>mcr p15, 0, r0, c7, c14, 1  @clean and invalidate single data cache 
> line
> diff --git