Re: [edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO
Thanks for your feedback. 1. Reg coding style, I will remove _ and resubmit but somehow PR CI seemed to pass for me (https://github.com/tianocore/edk2/pull/4636). 2. For size and ext discovery should I wait until your ext discovery patch is merged? 3. Thanks for catching the issue with SENVCFG. Will fix it in the next revision after #2 is addressed. On Mon, Jul 24, 2023 at 10:03 AM Sunil V L wrote: > Hi Dhaval, > > On Thu, Jul 13, 2023 at 03:03:31PM +0530, Dhaval wrote: > > From: Dhaval Sharma > > > > Implementing code to support Cache Management Operations > > (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs > > > > Notes: > > 1. CMO only supports block based Operations. Meaning complete > >cache flush/invd/clean Operations are not available. In that case > >we fallback on fence.i instructions. > > 2. Rely on the fact that platform init has initialized CMO and this > >implementation just checks if it is enabled. > > 3. In order to avoid compiler dependency injecting byte code. > > > > Test: > > 1. Ensured correct instructions are refelecting in asm > > 2. Able to boot platform with RiscVVirtQemu config > > 3. Not able to verify actual instruction in HW as Qemu ignores > > any actual cache operations. > > > > Cc: Ard Biesheuvel > > Cc: Jiewen Yao > > Cc: Jordan Justen > > Cc: Gerd Hoffmann > > Cc: Sunil V L > > Cc: Andrei Warkentin > > Signed-off-by: Dhaval Sharma > > --- > > > > Notes: > > v4: > > - Removed CMO specific directory in Base Lib > > - Implemented compiler independent code for CMO > > - Merged CMO implementation with fence.i > > - Added logic to confirm CMO is enabled > > > > MdePkg/Library/BaseLib/BaseLib.inf | 2 +- > > MdePkg/Include/Register/RiscV64/RiscVEncoding.h | 4 + > > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 > ++-- > > MdePkg/Library/BaseLib/RiscV64/FlushCache.S | 21 -- > > MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S | 64 +++ > > 5 files changed, 254 insertions(+), 37 deletions(-) > > > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf > b/MdePkg/Library/BaseLib/BaseLib.inf > > index 03c7b02e828b..53389389448c 100644 > > --- a/MdePkg/Library/BaseLib/BaseLib.inf > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf > > @@ -400,7 +400,7 @@ [Sources.RISCV64] > >RiscV64/RiscVCpuBreakpoint.S | GCC > >RiscV64/RiscVCpuPause.S | GCC > >RiscV64/RiscVInterrupt.S | GCC > > - RiscV64/FlushCache.S | GCC > > + RiscV64/RiscVCacheMgmt.S | GCC > >RiscV64/CpuScratch.S | GCC > >RiscV64/ReadTimer.S | GCC > >RiscV64/RiscVMmu.S| GCC > > diff --git a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > > index 5c2989b797bf..ea1493578bd5 100644 > > --- a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > > +++ b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > > @@ -85,6 +85,10 @@ > > /* Supervisor Configuration */ > > #define CSR_SENVCFG 0x10a > > > > +/* Defined CBO bits*/ > > +#define SENVCFG_CBCFE 0x40UL > > +#define SENVCFG_CBIE 0x30UL > > + > > /* Supervisor Trap Handling */ > > #define CSR_SSCRATCH 0x140 > > #define CSR_SEPC 0x141 > > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > index d08fb9f193ca..8b853e5b69fa 100644 > > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > @@ -1,7 +1,8 @@ > > /** @file > > - RISC-V specific functionality for cache. > > + Implement Risc-V Cache Management Operations > > > >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 > > **/ > > @@ -10,13 +11,21 @@ > > #include > > #include > > > > +#define RV64_CACHE_BLOCK_SIZE 64 > > + > Can we avoid hard coding this? We can get it from DT. > > > +typedef enum { > > + Clean, > > + Flush, > > + Invld, > > +} CACHE_OP; > > + > > /** > >RISC-V invalidate instruction cache. > > > > **/ > > VOID > > EFIAPI > > -RiscVInvalidateInstCacheAsm ( > > +RiscVInvalidateInstCacheAsm_Fence ( > This is not EDK2 coding style... and other similar functions below. > > >VOID > >); > > > > @@ -26,13 +35,144 @@ RiscVInvalidateInstCacheAsm ( > > **/ > > VOID > > EFIAPI > > -RiscVInvalidateDataCacheAsm ( > > +RiscVInvalidateDataCacheAsm_Fence ( > >VOID > >); > > > > +/** > > + RISC-V flush cache block. Atomically perform a clean operation > > + followed by an invalidate operation > > + > > +**/ > > +VOID > > +EFIAPI > > +RiscVCpuCacheFlushAsm_Cbo ( > > + UINTN > > + ); > > + > > +/** > > +Perform a write transfer to another cache or to memory if the > > +data in the copy of the cache
Re: [edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO
Hi Dhaval, On Thu, Jul 13, 2023 at 03:03:31PM +0530, Dhaval wrote: > From: Dhaval Sharma > > Implementing code to support Cache Management Operations > (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs > > Notes: > 1. CMO only supports block based Operations. Meaning complete >cache flush/invd/clean Operations are not available. In that case >we fallback on fence.i instructions. > 2. Rely on the fact that platform init has initialized CMO and this >implementation just checks if it is enabled. > 3. In order to avoid compiler dependency injecting byte code. > > Test: > 1. Ensured correct instructions are refelecting in asm > 2. Able to boot platform with RiscVVirtQemu config > 3. Not able to verify actual instruction in HW as Qemu ignores > any actual cache operations. > > Cc: Ard Biesheuvel > Cc: Jiewen Yao > Cc: Jordan Justen > Cc: Gerd Hoffmann > Cc: Sunil V L > Cc: Andrei Warkentin > Signed-off-by: Dhaval Sharma > --- > > Notes: > v4: > - Removed CMO specific directory in Base Lib > - Implemented compiler independent code for CMO > - Merged CMO implementation with fence.i > - Added logic to confirm CMO is enabled > > MdePkg/Library/BaseLib/BaseLib.inf | 2 +- > MdePkg/Include/Register/RiscV64/RiscVEncoding.h | 4 + > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 > ++-- > MdePkg/Library/BaseLib/RiscV64/FlushCache.S | 21 -- > MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S | 64 +++ > 5 files changed, 254 insertions(+), 37 deletions(-) > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf > b/MdePkg/Library/BaseLib/BaseLib.inf > index 03c7b02e828b..53389389448c 100644 > --- a/MdePkg/Library/BaseLib/BaseLib.inf > +++ b/MdePkg/Library/BaseLib/BaseLib.inf > @@ -400,7 +400,7 @@ [Sources.RISCV64] >RiscV64/RiscVCpuBreakpoint.S | GCC >RiscV64/RiscVCpuPause.S | GCC >RiscV64/RiscVInterrupt.S | GCC > - RiscV64/FlushCache.S | GCC > + RiscV64/RiscVCacheMgmt.S | GCC >RiscV64/CpuScratch.S | GCC >RiscV64/ReadTimer.S | GCC >RiscV64/RiscVMmu.S| GCC > diff --git a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > index 5c2989b797bf..ea1493578bd5 100644 > --- a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > +++ b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > @@ -85,6 +85,10 @@ > /* Supervisor Configuration */ > #define CSR_SENVCFG 0x10a > > +/* Defined CBO bits*/ > +#define SENVCFG_CBCFE 0x40UL > +#define SENVCFG_CBIE 0x30UL > + > /* Supervisor Trap Handling */ > #define CSR_SSCRATCH 0x140 > #define CSR_SEPC 0x141 > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > index d08fb9f193ca..8b853e5b69fa 100644 > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > @@ -1,7 +1,8 @@ > /** @file > - RISC-V specific functionality for cache. > + Implement Risc-V Cache Management Operations > >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 > **/ > @@ -10,13 +11,21 @@ > #include > #include > > +#define RV64_CACHE_BLOCK_SIZE 64 > + Can we avoid hard coding this? We can get it from DT. > +typedef enum { > + Clean, > + Flush, > + Invld, > +} CACHE_OP; > + > /** >RISC-V invalidate instruction cache. > > **/ > VOID > EFIAPI > -RiscVInvalidateInstCacheAsm ( > +RiscVInvalidateInstCacheAsm_Fence ( This is not EDK2 coding style... and other similar functions below. >VOID >); > > @@ -26,13 +35,144 @@ RiscVInvalidateInstCacheAsm ( > **/ > VOID > EFIAPI > -RiscVInvalidateDataCacheAsm ( > +RiscVInvalidateDataCacheAsm_Fence ( >VOID >); > > +/** > + RISC-V flush cache block. Atomically perform a clean operation > + followed by an invalidate operation > + > +**/ > +VOID > +EFIAPI > +RiscVCpuCacheFlushAsm_Cbo ( > + UINTN > + ); > + > +/** > +Perform a write transfer to another cache or to memory if the > +data in the copy of the cache block have been modified by a store > +operation > + > +**/ > +VOID > +EFIAPI > +RiscVCpuCacheCleanAsm_Cbo ( > + UINTN > + ); > + > +/** > +Deallocate the copy of the cache block > + > +**/ > +VOID > +EFIAPI > +RiscVCpuCacheInvalAsm_Cbo ( > + UINTN > + ); > + > +/** > +Verify CBOs are supported by this HW > +CBCFE == Cache Block Clean and Flush instruction Enable > +CBIE == Cache Block Invalidate instruction Enable > + > +**/ > +UINTN > +RiscvIsCbcfeEnabledAsm ( > + VOID > + ); > + > +UINTN > +RiscvIsCbiEnabledAsm ( > + VOID > + ); > + > +/** > + Performs required opeartion on cache lines in the cache coherency domain > + of the calling CPU. If
[edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO
From: Dhaval Sharma Implementing code to support Cache Management Operations (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs Notes: 1. CMO only supports block based Operations. Meaning complete cache flush/invd/clean Operations are not available. In that case we fallback on fence.i instructions. 2. Rely on the fact that platform init has initialized CMO and this implementation just checks if it is enabled. 3. In order to avoid compiler dependency injecting byte code. Test: 1. Ensured correct instructions are refelecting in asm 2. Able to boot platform with RiscVVirtQemu config 3. Not able to verify actual instruction in HW as Qemu ignores any actual cache operations. Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Jordan Justen Cc: Gerd Hoffmann Cc: Sunil V L Cc: Andrei Warkentin Signed-off-by: Dhaval Sharma --- Notes: v4: - Removed CMO specific directory in Base Lib - Implemented compiler independent code for CMO - Merged CMO implementation with fence.i - Added logic to confirm CMO is enabled MdePkg/Library/BaseLib/BaseLib.inf | 2 +- MdePkg/Include/Register/RiscV64/RiscVEncoding.h | 4 + MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 ++-- MdePkg/Library/BaseLib/RiscV64/FlushCache.S | 21 -- MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S | 64 +++ 5 files changed, 254 insertions(+), 37 deletions(-) diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf index 03c7b02e828b..53389389448c 100644 --- a/MdePkg/Library/BaseLib/BaseLib.inf +++ b/MdePkg/Library/BaseLib/BaseLib.inf @@ -400,7 +400,7 @@ [Sources.RISCV64] RiscV64/RiscVCpuBreakpoint.S | GCC RiscV64/RiscVCpuPause.S | GCC RiscV64/RiscVInterrupt.S | GCC - RiscV64/FlushCache.S | GCC + RiscV64/RiscVCacheMgmt.S | GCC RiscV64/CpuScratch.S | GCC RiscV64/ReadTimer.S | GCC RiscV64/RiscVMmu.S| GCC diff --git a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h index 5c2989b797bf..ea1493578bd5 100644 --- a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h +++ b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h @@ -85,6 +85,10 @@ /* Supervisor Configuration */ #define CSR_SENVCFG 0x10a +/* Defined CBO bits*/ +#define SENVCFG_CBCFE 0x40UL +#define SENVCFG_CBIE 0x30UL + /* Supervisor Trap Handling */ #define CSR_SSCRATCH 0x140 #define CSR_SEPC 0x141 diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c index d08fb9f193ca..8b853e5b69fa 100644 --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c @@ -1,7 +1,8 @@ /** @file - RISC-V specific functionality for cache. + Implement Risc-V Cache Management Operations 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 **/ @@ -10,13 +11,21 @@ #include #include +#define RV64_CACHE_BLOCK_SIZE 64 + +typedef enum { + Clean, + Flush, + Invld, +} CACHE_OP; + /** RISC-V invalidate instruction cache. **/ VOID EFIAPI -RiscVInvalidateInstCacheAsm ( +RiscVInvalidateInstCacheAsm_Fence ( VOID ); @@ -26,13 +35,144 @@ RiscVInvalidateInstCacheAsm ( **/ VOID EFIAPI -RiscVInvalidateDataCacheAsm ( +RiscVInvalidateDataCacheAsm_Fence ( VOID ); +/** + RISC-V flush cache block. Atomically perform a clean operation + followed by an invalidate operation + +**/ +VOID +EFIAPI +RiscVCpuCacheFlushAsm_Cbo ( + UINTN + ); + +/** +Perform a write transfer to another cache or to memory if the +data in the copy of the cache block have been modified by a store +operation + +**/ +VOID +EFIAPI +RiscVCpuCacheCleanAsm_Cbo ( + UINTN + ); + +/** +Deallocate the copy of the cache block + +**/ +VOID +EFIAPI +RiscVCpuCacheInvalAsm_Cbo ( + UINTN + ); + +/** +Verify CBOs are supported by this HW +CBCFE == Cache Block Clean and Flush instruction Enable +CBIE == Cache Block Invalidate instruction Enable + +**/ +UINTN +RiscvIsCbcfeEnabledAsm ( + VOID + ); + +UINTN +RiscvIsCbiEnabledAsm ( + VOID + ); + +/** + 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. If the CPU is in a physical addressing mode, + then Address is a physical address. If the CPU is in a virtual + addressing