Rv64 cmo support review #2
Conversation
Adding support for build time switch to select CMO support Requires GCC Binutils version 2.39 onwards and CPU support Cc: Sunil V L <[email protected]> Signed-off-by: Dhaval <[email protected]>
Implement CMO according to RiscV Zicbo ext specifications Cc: Sunil V L <[email protected]> Signed-off-by: Dhaval <[email protected]>
Fixed minor patch check issues Cc: Sunil V L <[email protected]> Signed-off-by: Dhaval <[email protected]>
| #define __stringify(x...) __stringify_1(x) | ||
|
|
||
| #define CMO_OP(_op, _start)\ | ||
| asm volatile("cbo." __stringify(_op) " (%0)" :: "r" (Start)) |
There was a problem hiding this comment.
inline assembly is discouraged in edk2. Could you convert it into assembly functions?
There was a problem hiding this comment.
Fixed. Also it seems like we will require to do refactoring in BaseLib.h when your first set of refactored patches are checked in.
There was a problem hiding this comment.
What kind of refactoring do we need? Do you mean splitting the header file or the library?
There was a problem hiding this comment.
Please check my comments in /RiscVCache.c
There was a problem hiding this comment.
But why is it dependent on refactoring work? BaseLib already has RiscV64/FlushCache.S, right? Why can't these new assembly routines be added there?
There was a problem hiding this comment.
If you compare x86cache.c and Riscvcache.c: x86cache uses AsmWbinvd declared in Baselib.h and implemented in MdePkg/Baselib. Idea was probably that Baselib will have all common functions to manage cache. But RV has slightly different functionality so we can not use same baselib.h declarations and instead I am linking them directly back into MdePkh/Baselib (without using baselib.h)
There was a problem hiding this comment.
During refactoring if we can consider AsmFlushCacheLine and other relevant cache management APIs considering RV, then it will be easy to maintain the same links.
Implement CMO according to RiscV Zicbo ext specifications Cc: Sunil V L <[email protected]> Signed-off-by: Dhaval <[email protected]>
Fix comments for CMO functions Cc: Sunil V L <[email protected]> Signed-off-by: Dhaval <[email protected]>
| # GCC:Binutils 2.39 is required. We could make it runtime detection later | ||
| # using FDT or feature CSR. | ||
| # | ||
| DEFINE RV_CMO_FEATURE_AVAILABLE = FALSE |
There was a problem hiding this comment.
Thanks. Unfortunately, there is no easy way to detect the binutils capabilities at the runtime in edk2. So, we will have to move to support only toolchain versions having > 2.39 sometime soon. A generic infrastructure to read the extensions from the FDT is better so that it works for any extension.
Initial patches to start reviewing Zicbom support. Still there are some opens around
Actual CMO enabling (CBIE programming in SVENCFG)
Platform specific support (some platforms may support entire cache eviction but through MMIO config. Need to see how to support that.
Runtime detection and enabling of the feature.