Ard, This is probably a reflection of my lack of deep understanding of ARM cache maintenance on more complex systems.
The "entire cache" operations exist to allow transitions across major environments (initialization of non-self initializing caches, changing phases of boot, etc) to clean/clean+invalidate/invalidate cache without knowing apriori what virtual addresses have dirty data. The set/way solution was a matter of convenience since the ARM did not have an architectural means of operating on the full cache in one instruction. We do have cases where we need to turn off caches entirely - booting our OS as well as in core power management scenarios - I think we have to do a full cache flush in these cases if not by set/way then somehow. I don't think we can expect any one component (or whatever phase-thunking code) to know what virtual addresses may have dirty data nor can we expect it to loop across all possible cacheable virtual addresses. I also see that the "threshold" based modification of the cache flush from MVA to entire-cache was removed earlier this year (I missed that!). Have you quantified the performance impact from removing of the threshold methodology? Since PEI/DXE/UEFI Boot Services is uniprocessor I don't know if it makes sense to eliminate the "entire cache" methodology entirely - I agree for other phases like runtime and TZ/SMM that it should be banished. Eugene -----Original Message----- From: Ard Biesheuvel [mailto:[email protected]] Sent: Tuesday, November 03, 2015 7:30 AM To: Cohen, Eugene <[email protected]> Cc: [email protected]; [email protected]; [email protected]; [email protected] Subject: Re: [edk2] [PATCH 08/10] ArmCacheMaintenanceLib: disallow whole D-cache maintenance operations On 3 November 2015 at 14:51, Cohen, Eugene <[email protected]> wrote: > Please don't remove this functionality. At times we do want to use this > library to turn off the cache in preparation for going to another environment > (say, loading an OS) and this is useful. > Note that this is not about turning off the caches. As Mark points out, the set/way operations are not suitable for ensuring that the OS boot image has made it all the way to memory, and that it will be visible to the CPU with the MMU off. The cache maintenance by VA operations do offer those guarantees, so the architecturally sound method of achieving that is to clean the region(s) you care about by VA before disabling the MMU. Under virtualization, these issues are a bit more prominent, and I have had to add by-VA cache maintenance on several occasions to fix actual problems. Note that this patch does not remove any of the functionality. It just changes the CacheMaintenanceLib implementation so that this functionality is not exported to generic code. If any non-ARM specific code in the tree requires the ability to clean and/or invalidate the whole data cache, it is making incorrect assumptions about the platform that it runs on. Regards, Ard. > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Ard Biesheuvel > Sent: Tuesday, November 03, 2015 3:17 AM > To: [email protected]; [email protected]; > [email protected] > Cc: [email protected]; Ard Biesheuvel <[email protected]> > Subject: [edk2] [PATCH 08/10] ArmCacheMaintenanceLib: disallow whole > D-cache maintenance operations > > The ARM architecture provides no reliable way to clean or invalidate the > entire data cache at runtime. The reason is that such maintenance requires > the use of set/way maintenance operations, which are suitable only for the > kind of maintenance that is carried out when the cache is taken offline > entirely. > > So ASSERT () when any of the CacheMaintenanceLib whole data cache routines > are invoked rather than pretending we can do anything meaningful here. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 8 > ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git > a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c > b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c > index 175d29496c32..8cc1990b3266 100644 > --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c > +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c > @@ -14,6 +14,7 @@ > **/ > #include <Base.h> > #include <Library/ArmLib.h> > +#include <Library/DebugLib.h> > #include <Library/PcdLib.h> > > VOID > @@ -44,7 +45,6 @@ InvalidateInstructionCache ( > VOID > ) > { > - ArmCleanDataCache(); > ArmInvalidateInstructionCache(); > } > > @@ -54,7 +54,7 @@ InvalidateDataCache ( > VOID > ) > { > - ArmInvalidateDataCache(); > + ASSERT (FALSE); > } > > VOID * > @@ -76,7 +76,7 @@ WriteBackInvalidateDataCache ( > VOID > ) > { > - ArmCleanInvalidateDataCache(); > + ASSERT (FALSE); > } > > VOID * > @@ -96,7 +96,7 @@ WriteBackDataCache ( > VOID > ) > { > - ArmCleanDataCache(); > + ASSERT (FALSE); > } > > VOID * > -- > 1.9.1 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
