On 20 November 2015 at 10:29, Heyi Guo <[email protected]> wrote: > Hi Ard, > > Do we have special reason of not cleaning the allocated pages? If not > having, I think it is safer to use "clean and invalidate". >
Actually, I disagree. If your firmware relies on data making it to main memory only due to a subsequent clean performed by the next owner of the memory range, I'd much rather make it explicit. And clean + invalidate is obviously more costly. Leif? > On 11/20/2015 02:25 PM, Ard Biesheuvel wrote: >> >> On 20 November 2015 at 06:32, Heyi Guo <[email protected]> wrote: >>> >>> It is implied that the memory returned from UncachedMemoryAllocationLib >>> should have cache cleaned. So we clean and invalidate memory range after >>> changing memory attribute to uncached. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Heyi Guo <[email protected]> >>> Cc: Leif Lindholm <[email protected]> >>> Cc: Ard Biesheuvel <[email protected]> >>> --- >>> .../Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c >>> | 3 +++ >>> .../UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf >>> | 1 + >>> 2 files changed, 4 insertions(+) >>> >>> diff --git >>> a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c >>> b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c >>> index b859f63..3242579 100644 >>> --- >>> a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c >>> +++ >>> b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c >>> @@ -25,6 +25,7 @@ >>> #include <Library/PcdLib.h> >>> #include <Library/ArmLib.h> >>> #include <Library/DxeServicesTableLib.h> >>> +#include <Library/CacheMaintenanceLib.h> >>> >>> VOID * >>> UncachedInternalAllocatePages ( >>> @@ -165,6 +166,8 @@ AllocatePagesFromList ( >>> return Status; >>> } >>> >>> + WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)Memory, >>> EFI_PAGES_TO_SIZE (Pages)); >>> + >> >> I think invalidate only should be fine here, since it is a new >> allocation, and we are allocating full pages. Since the architectural >> max value of CWG is 2 KB, we could never end up discarding data that >> is not covered by the allocation itself. >> >>> NewNode = AllocatePool (sizeof (FREE_PAGE_NODE)); >>> if (NewNode == NULL) { >>> ASSERT (FALSE); >>> diff --git >>> a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf >>> b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf >>> index 0a0b6cb..d7a0f2f 100644 >>> --- >>> a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf >>> +++ >>> b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf >>> @@ -38,6 +38,7 @@ >>> MemoryAllocationLib >>> PcdLib >>> DxeServicesTableLib >>> + CacheMaintenanceLib >>> >>> [Pcd] >>> gArmTokenSpaceGuid.PcdArmFreeUncachedMemorySizeThreshold >>> -- >>> 2.6.2 >>> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
