On 11 May 2016 at 12:22, Mark Rutland <[email protected]> wrote: > On Wed, May 11, 2016 at 12:07:51PM +0200, Ard Biesheuvel wrote: >> On 11 May 2016 at 11:35, Achin Gupta <[email protected]> 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
