Thanks. Just to clarify, In the earlier formatting "InvalidateDataCacheRange:\ > + Zicbom not supported.\n" \ A missing " after *Range:\ was causing slightly skewed prints. After adding this " it looks okay. So that is one change I had addressed. But if keeping it in a single line works better please feel free to update. And Thanks! On Tue, Dec 19, 2023 at 12:59 PM Sunil V L wrote: > On Wed, Dec 13, 2023 at 08:29:30PM +0530, Dhaval Sharma wrote: > > Use newly defined cache management operations for RISC-V where possible > > It builds up on the support added for RISC-V cache management > > instructions in BaseLib. > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Zhiguang Liu > > Cc: Laszlo Ersek > > Cc: Pedro Falcato > > > > Signed-off-by: Dhaval Sharma > > Acked-by: Laszlo Ersek > > Reviewed-by: Pedro Falcato > > --- > > > > Notes: > > V10: > > - Fix formatting to keep comments within 80 > > - Replace RV with RISC-V > > - Fix an issue with multi line comments > > - Added assert to an unsupported function > > - Minor case modification in str in .uni > > > > V9: > > - Fixed an issue with Instruction cache invalidation. Use fence.i > > instruction as CMO does not support i-cache operations. > > V8: > > - Added note to convert PCD into RISC-V feature bitmap pointer > > - Modified function names to be more explicit about cache ops > > - Added RB tag > > V7: > > - Added PcdLib > > - Restructure DEBUG message based on feedback on V6 > > - Make naming consistent to CMO, remove all CBO references > > - Add ASSERT for not supported functions instead of plain debug > message > > - Added RB tag > > V6: > > - Utilize cache management instructions if HW supports it > > This patch is part of restructuring on top of v5 > > > > MdePkg/MdePkg.dec | > 8 + > > MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | > 5 + > > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | > 177 ++++++++++++++++---- > > MdePkg/MdePkg.uni | > 4 + > > 4 files changed, 166 insertions(+), 28 deletions(-) > > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > > index ac54338089e8..fa92673ff633 100644 > > --- a/MdePkg/MdePkg.dec > > +++ b/MdePkg/MdePkg.dec > > @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, > PcdsPatchableInModule.AARCH64] > > # @Prompt CPU Rng algorithm's GUID. > > > gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00000037 > > > > +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] > > + # > > + # Configurability to override RISC-V CPU Features > > + # BIT 0 = Cache Management Operations. This bit is relevant only if > > + # previous stage has feature enabled and user wants to disable it. > > + # > > + > gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0xFFFFFFFFFFFFFFFF|UINT64|0x69 > > + > > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > > ## This value is used to set the base address of PCI express > hierarchy. > > # @Prompt PCI Express Base Address. > > diff --git > a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > > index 6fd9cbe5f6c9..601a38d6c109 100644 > > --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > > @@ -56,3 +56,8 @@ [LibraryClasses] > > BaseLib > > DebugLib > > > > +[LibraryClasses.RISCV64] > > + PcdLib > > + > > +[Pcd.RISCV64] > > + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES > > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > index ac2a3c23a249..7c53a17abbb5 100644 > > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > @@ -2,6 +2,7 @@ > > RISC-V specific functionality for cache. > > > > Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All > rights reserved.
> > + Copyright (c) 2023, Rivos Inc. All rights reserved.
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > @@ -9,10 +10,116 @@ > > #include > > #include > > #include > > +#include > > + > > +// > > +// TODO: Grab cache block size and make Cache Management Operation > > +// enabling decision based on RISC-V CPU HOB in > > +// future when it is available and convert PcdRiscVFeatureOverride > > +// PCD to a pointer that contains pointer to bitmap structure > > +// which can be operated more elegantly. > > +// > > +#define RISCV_CACHE_BLOCK_SIZE 64 > > +#define RISCV_CPU_FEATURE_CMO_BITMASK 0x1 > > + > > +typedef enum { > > + CacheOpClean, > > + CacheOpFlush, > > + CacheOpInvld, > > +} CACHE_OP; > > + > > +/** > > +Verify CBOs are supported by this HW > > +TODO: Use RISC-V CPU HOB once available. > > + > > +**/ > > +STATIC > > +BOOLEAN > > +RiscVIsCMOEnabled ( > > + VOID > > + ) > > +{ > > + // If CMO is disabled in HW, skip Override check > > + // Otherwise this PCD can override settings > > + return ((PcdGet64 (PcdRiscVFeatureOverride) & > RISCV_CPU_FEATURE_CMO_BITMASK) != 0); > > +} > > + > > +/** > > + Performs required opeartion on cache lines in the cache coherency > domain > > + of the calling CPU. If Address is not aligned on a cache line > boundary, > > + then entire cache line containing Address is operated. If Address + > Length > > + is not aligned on a cache line boundary, then the entire cache line > > + containing Address + Length -1 is operated. > > + If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT(). > > + @param Address The base address of the cache lines to > > + invalidate. > > + @param Length The number of bytes to invalidate from the instruction > > + cache. > > + @param Op Type of CMO operation to be performed > > + @return Address. > > + > > +**/ > > +STATIC > > +VOID > > +CacheOpCacheRange ( > > + IN VOID *Address, > > + IN UINTN Length, > > + IN CACHE_OP Op > > + ) > > +{ > > + UINTN CacheLineSize; > > + UINTN Start; > > + UINTN End; > > + > > + if (Length == 0) { > > + return; > > + } > > + > > + if ((Op != CacheOpInvld) && (Op != CacheOpFlush) && (Op != > CacheOpClean)) { > > + return; > > + } > > + > > + ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Address)); > > + > > + CacheLineSize = RISCV_CACHE_BLOCK_SIZE; > > + > > + Start = (UINTN)Address; > > + // > > + // Calculate the cache line alignment > > + // > > + End = (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize - > 1); > > + Start &= ~((UINTN)CacheLineSize - 1); > > + > > + DEBUG ( > > + (DEBUG_VERBOSE, > > + "CacheOpCacheRange: Performing Cache Management Operation %d \n", > Op) > > + ); > > + > > + do { > > + switch (Op) { > > + case CacheOpInvld: > > + RiscVCpuCacheInvalCmoAsm (Start); > > + break; > > + case CacheOpFlush: > > + RiscVCpuCacheFlushCmoAsm (Start); > > + break; > > + case CacheOpClean: > > + RiscVCpuCacheCleanCmoAsm (Start); > > + break; > > + default: > > + break; > > + } > > + > > + Start = Start + CacheLineSize; > > + } while (Start != End); > > +} > > > > /** > > Invalidates the entire instruction cache in cache coherency domain of > the > > - calling CPU. > > + calling CPU. Risc-V does not have currently an CBO implementation > which can > > + invalidate the entire I-cache. Hence using Fence instruction for now. > P.S. > > + Fence instruction may or may not implement full I-cache invd > functionality > > + on all implementations. > > > > **/ > > VOID > > @@ -28,17 +135,11 @@ InvalidateInstructionCache ( > > Invalidates a range of instruction cache lines in the cache coherency > domain > > of the calling CPU. > > > > - Invalidates the instruction cache lines specified by Address and > Length. If > > - Address is not aligned on a cache line boundary, then entire > instruction > > - cache line containing Address is invalidated. If Address + Length is > not > > - aligned on a cache line boundary, then the entire instruction cache > line > > - containing Address + Length -1 is invalidated. This function may > choose to > > - invalidate the entire instruction cache if that is more efficient than > > - invalidating the specified range. If Length is 0, then no instruction > cache > > - lines are invalidated. Address is returned. > > - > > - If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT(). > > - > > + An operation from a CMO instruction is defined to operate only on the > copies > > + of a cache block that are cached in the caches accessible by the > explicit > > + memory accesses performed by the set of coherent agents.In other > words CMO > > + operations are not applicable to instruction cache. Use fence.i > instruction > > + instead to achieve the same purpose. > > @param Address The base address of the instruction cache lines to > > invalidate. If the CPU is in a physical addressing > mode, then > > Address is a physical address. If the CPU is in a > virtual > > @@ -57,9 +158,10 @@ InvalidateInstructionCacheRange ( > > ) > > { > > DEBUG ( > > - (DEBUG_WARN, > > - "%a:RISC-V unsupported function.\n" > > - "Invalidating the whole instruction cache instead.\n", __func__) > > + (DEBUG_VERBOSE, > > + "InvalidateInstructionCacheRange: RISC-V unsupported function.\n" > > + "Invalidating the whole instruction cache instead.\n" > > + ) > > ); > > InvalidateInstructionCache (); > > return Address; > > @@ -81,7 +183,12 @@ WriteBackInvalidateDataCache ( > > VOID > > ) > > { > > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__)); > > + ASSERT (FALSE); > > + DEBUG (( > > + DEBUG_ERROR, > > + "WriteBackInvalidateDataCache:" \ > > + "RISC-V unsupported function.\n" > I guess this formatting was pointed by Pedro earlier. This can be made > single line. Let me fix it before merging so that you don't need to send > another version. > > Reviewed-by: Sunil V L > -- Thanks! =D -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112713): https://edk2.groups.io/g/devel/message/112713 Mute This Topic: https://groups.io/mt/103150435/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-