[PATCH v4 0/1] MdePkg:Implement RISCV CMO

[PATCH v4 0/1] MdePkg:Implement RISCV CMO

[Dhaval Sharma]

Implementing code to support Cache Management Operations (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs

Notes:

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.
Rely on the fact that platform init has initialized CMO and this implementation just checks if it is enabled.
In order to avoid compiler dependency injecting byte code.
Test:

Ensured correct instructions are refelecting in asm
Able to boot platform with RiscVVirtQemu config
Not able to verify actual instruction in HW as Qemu ignores any actual cache operations.

Dhaval Sharma (1):
  MdePkg:Implement RISCV CMO

 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(-)
 delete mode 100644 MdePkg/Library/BaseLib/RiscV64/FlushCache.S
 create mode 100644 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S

-- 
2.34.1

[PATCH v4 1/1] MdePkg:Implement RISCV CMO

[Dhaval Sharma]

From: Dhaval Sharma <dhaval@rivosinc.com>

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 <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Andrei Warkentin <andrei.warkentin@intel.com>
Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
---

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.<BR>
+  Copyright (c) 2023, Rivos Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -10,13 +11,21 @@
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 
+#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 mode, then Address is a virtual address.
+
+  @param  Length  The number of bytes to invalidate from the instruction
+          cache.
+
+  @param  Op  Type of CMO operation to be performed
+
+  @return Address.
+
+**/
+VOID *
+EFIAPI
+CacheOpCacheRange (
+  IN VOID      *Address,
+  IN UINTN     Length,
+  IN CACHE_OP  Op
+  )
+{
+  UINTN  CacheLineSize;
+  UINTN  Start;
+  UINTN  End;
+
+  if (Length == 0) {
+    return Address;
+  }
+
+  ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Address));
+
+  //
+  // Cache line size is 8 * Bits 15-08 of EBX returned from CPUID 01H
+  //
+  CacheLineSize = RV64_CACHE_BLOCK_SIZE;
+
+  Start = (UINTN)Address;
+  //
+  // Calculate the cache line alignment
+  //
+  End    = (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize - 1);
+  Start &= ~((UINTN)CacheLineSize - 1);
+
+  DEBUG (
+    (DEBUG_INFO,
+     "%a Performing Cache Management Operation %d \n", __func__, Op)
+    );
+
+  do {
+    switch (Op) {
+      case Invld:
+        RiscVCpuCacheInvalAsm_Cbo (Start);
+        break;
+      case Flush:
+        RiscVCpuCacheFlushAsm_Cbo (Start);
+        break;
+      case Clean:
+        RiscVCpuCacheCleanAsm_Cbo (Start);
+        break;
+      default:
+        DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported operation\n"));
+        break;
+    }
+
+    Start = Start + CacheLineSize;
+  } while (Start != End);
+
+  return Address;
+}
+
 /**
   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 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
@@ -41,7 +181,7 @@ InvalidateInstructionCache (
   VOID
   )
 {
-  RiscVInvalidateInstCacheAsm ();
+  RiscVInvalidateInstCacheAsm_Fence ();
 }
 
 /**
@@ -76,12 +216,17 @@ InvalidateInstructionCacheRange (
   IN UINTN  Length
   )
 {
-  DEBUG (
-    (DEBUG_WARN,
-     "%a:RISC-V unsupported function.\n"
-     "Invalidating the whole instruction cache instead.\n", __func__)
-    );
-  InvalidateInstructionCache ();
+  if (RiscvIsCbiEnabledAsm () == RETURN_SUCCESS) {
+    CacheOpCacheRange (Address, Length, Invld);
+  } else {
+    DEBUG (
+      (DEBUG_WARN,
+       "%a:RISC-V unsupported function.\n"
+       "Invalidating the whole instruction cache instead.\n", __func__)
+      );
+    InvalidateInstructionCache ();
+  }
+
   return Address;
 }
 
@@ -137,7 +282,12 @@ WriteBackInvalidateDataCacheRange (
   IN      UINTN  Length
   )
 {
-  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
+  if (RiscvIsCbcfeEnabledAsm () == RETURN_SUCCESS) {
+    CacheOpCacheRange (Address, Length, Flush);
+  } else {
+    DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
+  }
+
   return Address;
 }
 
@@ -192,7 +342,12 @@ WriteBackDataCacheRange (
   IN      UINTN  Length
   )
 {
-  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
+  if (RiscvIsCbcfeEnabledAsm () == RETURN_SUCCESS) {
+    CacheOpCacheRange (Address, Length, Clean);
+  } else {
+    DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
+  }
+
   return Address;
 }
 
@@ -213,7 +368,12 @@ InvalidateDataCache (
   VOID
   )
 {
-  RiscVInvalidateDataCacheAsm ();
+  DEBUG (
+    (DEBUG_WARN,
+     "%a:RISC-V unsupported function.\n"
+     "Invalidating the whole instruction cache instead.\n", __func__)
+    );
+  RiscVInvalidateDataCacheAsm_Fence ();
 }
 
 /**
@@ -250,6 +410,16 @@ InvalidateDataCacheRange (
   IN      UINTN  Length
   )
 {
-  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
+  if (RiscvIsCbiEnabledAsm () == RETURN_SUCCESS) {
+    CacheOpCacheRange (Address, Length, Invld);
+  } else {
+    DEBUG (
+      (DEBUG_WARN,
+       "%a:RISC-V unsupported function.\n"
+       "Invalidating the whole instruction cache instead.\n", __func__)
+      );
+    InvalidateDataCache ();
+  }
+
   return Address;
 }
diff --git a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S b/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
deleted file mode 100644
index 7c10fdd268af..000000000000
--- a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
+++ /dev/null
@@ -1,21 +0,0 @@
-//------------------------------------------------------------------------------
-//
-// RISC-V cache operation.
-//
-// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
-//
-// SPDX-License-Identifier: BSD-2-Clause-Patent
-//
-//------------------------------------------------------------------------------
-
-.align 3
-ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm)
-ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm)
-
-ASM_PFX(RiscVInvalidateInstCacheAsm):
-    fence.i
-    ret
-
-ASM_PFX(RiscVInvalidateDataCacheAsm):
-    fence
-    ret
diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
new file mode 100644
index 000000000000..ecf391632221
--- /dev/null
+++ b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
@@ -0,0 +1,64 @@
+//------------------------------------------------------------------------------
+//
+// RISC-V cache operation.
+//
+// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
+// Copyright (c) 2022, Rivos Inc. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//------------------------------------------------------------------------------
+#include <Register/RiscV64/RiscVImpl.h>
+
+.align 3
+ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm_Fence)
+ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm_Fence)
+
+ASM_PFX(RiscVInvalidateInstCacheAsm_Fence):
+    fence.i
+    ret
+
+ASM_PFX(RiscVInvalidateDataCacheAsm_Fence):
+    fence
+    ret
+
+ASM_GLOBAL ASM_PFX (RiscVCpuCacheFlushAsm_Cbo)
+ASM_PFX (RiscVCpuCacheFlushAsm_Cbo):
+
+  .long 0x0025200f
+  ret
+
+ASM_GLOBAL ASM_PFX (RiscVCpuCacheCleanAsm_Cbo)
+ASM_PFX (RiscVCpuCacheCleanAsm_Cbo):
+  .long 0x0015200f
+  ret
+
+ASM_GLOBAL ASM_PFX (RiscVCpuCacheInvalAsm_Cbo)
+ASM_PFX (RiscVCpuCacheInvalAsm_Cbo):
+  .long 0x0005200f
+  ret
+
+ASM_GLOBAL ASM_PFX(RiscvIsCbiEnabledAsm)
+ASM_PFX(RiscvIsCbiEnabledAsm):
+    li    a0, 3
+    csrr  a1, CSR_SENVCFG
+    and   a1, a1, SENVCFG_CBIE
+    beqz  a1, skip
+
+    and   a1, a1, 0x10
+    beqz  a1, skip
+
+    mv    a0, x0
+skip:
+    ret
+
+ASM_GLOBAL ASM_PFX(RiscvIsCbcfeEnabledAsm)
+ASM_PFX(RiscvIsCbcfeEnabledAsm):
+    li    a0, 3
+    csrr  a1, CSR_SENVCFG
+    and   a1, a1, SENVCFG_CBCFE
+    beqz  a1, next
+
+    mv    a0, x0
+next:
+    ret
-- 
2.34.1

Re: [edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO

[Sunil V L]

Hi Dhaval,

On Thu, Jul 13, 2023 at 03:03:31PM +0530, Dhaval wrote:
> From: Dhaval Sharma <dhaval@rivosinc.com>
> 
> 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 <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: Andrei Warkentin <andrei.warkentin@intel.com>
> Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> ---
> 
> 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.<BR>
> +  Copyright (c) 2023, Rivos Inc. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> @@ -10,13 +11,21 @@
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  
> +#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 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 mode, then Address is a virtual address.
> +
> +  @param  Length  The number of bytes to invalidate from the instruction
> +          cache.
> +
> +  @param  Op  Type of CMO operation to be performed
> +
> +  @return Address.
> +
> +**/
> +VOID *
> +EFIAPI
> +CacheOpCacheRange (
> +  IN VOID      *Address,
> +  IN UINTN     Length,
> +  IN CACHE_OP  Op
> +  )
> +{
> +  UINTN  CacheLineSize;
> +  UINTN  Start;
> +  UINTN  End;
> +
> +  if (Length == 0) {
> +    return Address;
> +  }
> +
> +  ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Address));
> +
> +  //
> +  // Cache line size is 8 * Bits 15-08 of EBX returned from CPUID 01H
> +  //
This comments is invalid for RISC-V.

> +  CacheLineSize = RV64_CACHE_BLOCK_SIZE;
> +
> +  Start = (UINTN)Address;
> +  //
> +  // Calculate the cache line alignment
> +  //
> +  End    = (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize - 1);
> +  Start &= ~((UINTN)CacheLineSize - 1);
> +
> +  DEBUG (
> +    (DEBUG_INFO,
> +     "%a Performing Cache Management Operation %d \n", __func__, Op)
> +    );
> +
> +  do {
> +    switch (Op) {
> +      case Invld:
> +        RiscVCpuCacheInvalAsm_Cbo (Start);
> +        break;
> +      case Flush:
> +        RiscVCpuCacheFlushAsm_Cbo (Start);
> +        break;
> +      case Clean:
> +        RiscVCpuCacheCleanAsm_Cbo (Start);
> +        break;
> +      default:
> +        DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported operation\n"));
> +        break;
> +    }
> +
> +    Start = Start + CacheLineSize;
> +  } while (Start != End);
> +
> +  return Address;
> +}
> +
>  /**
>    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 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
> @@ -41,7 +181,7 @@ InvalidateInstructionCache (
>    VOID
>    )
>  {
> -  RiscVInvalidateInstCacheAsm ();
> +  RiscVInvalidateInstCacheAsm_Fence ();
>  }
>  
>  /**
> @@ -76,12 +216,17 @@ InvalidateInstructionCacheRange (
>    IN UINTN  Length
>    )
>  {
> -  DEBUG (
> -    (DEBUG_WARN,
> -     "%a:RISC-V unsupported function.\n"
> -     "Invalidating the whole instruction cache instead.\n", __func__)
> -    );
> -  InvalidateInstructionCache ();
> +  if (RiscvIsCbiEnabledAsm () == RETURN_SUCCESS) {
> +    CacheOpCacheRange (Address, Length, Invld);
> +  } else {
> +    DEBUG (
> +      (DEBUG_WARN,
> +       "%a:RISC-V unsupported function.\n"
> +       "Invalidating the whole instruction cache instead.\n", __func__)
> +      );
> +    InvalidateInstructionCache ();
> +  }
> +
>    return Address;
>  }
>  
> @@ -137,7 +282,12 @@ WriteBackInvalidateDataCacheRange (
>    IN      UINTN  Length
>    )
>  {
> -  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> +  if (RiscvIsCbcfeEnabledAsm () == RETURN_SUCCESS) {
> +    CacheOpCacheRange (Address, Length, Flush);
> +  } else {
> +    DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> +  }
> +
>    return Address;
>  }
>  
> @@ -192,7 +342,12 @@ WriteBackDataCacheRange (
>    IN      UINTN  Length
>    )
>  {
> -  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> +  if (RiscvIsCbcfeEnabledAsm () == RETURN_SUCCESS) {
> +    CacheOpCacheRange (Address, Length, Clean);
> +  } else {
> +    DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> +  }
> +
>    return Address;
>  }
>  
> @@ -213,7 +368,12 @@ InvalidateDataCache (
>    VOID
>    )
>  {
> -  RiscVInvalidateDataCacheAsm ();
> +  DEBUG (
> +    (DEBUG_WARN,
> +     "%a:RISC-V unsupported function.\n"
> +     "Invalidating the whole instruction cache instead.\n", __func__)
Data cache?
> +    );
> +  RiscVInvalidateDataCacheAsm_Fence ();
>  }
>  
>  /**
> @@ -250,6 +410,16 @@ InvalidateDataCacheRange (
>    IN      UINTN  Length
>    )
>  {
> -  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> +  if (RiscvIsCbiEnabledAsm () == RETURN_SUCCESS) {
> +    CacheOpCacheRange (Address, Length, Invld);
> +  } else {
> +    DEBUG (
> +      (DEBUG_WARN,
> +       "%a:RISC-V unsupported function.\n"
> +       "Invalidating the whole instruction cache instead.\n", __func__)
Data cache?

> +      );
> +    InvalidateDataCache ();
> +  }
> +
>    return Address;
>  }
> diff --git a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S b/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> deleted file mode 100644
> index 7c10fdd268af..000000000000
> --- a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -//------------------------------------------------------------------------------
> -//
> -// RISC-V cache operation.
> -//
> -// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> -//
> -// SPDX-License-Identifier: BSD-2-Clause-Patent
> -//
> -//------------------------------------------------------------------------------
> -
> -.align 3
> -ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm)
> -ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm)
> -
> -ASM_PFX(RiscVInvalidateInstCacheAsm):
> -    fence.i
> -    ret
> -
> -ASM_PFX(RiscVInvalidateDataCacheAsm):
> -    fence
> -    ret
Have you done git mv first to rename the file so that git history is
maintained?

> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> new file mode 100644
> index 000000000000..ecf391632221
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> @@ -0,0 +1,64 @@
> +//------------------------------------------------------------------------------
> +//
> +// RISC-V cache operation.
> +//
> +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +// Copyright (c) 2022, Rivos Inc. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//------------------------------------------------------------------------------
> +#include <Register/RiscV64/RiscVImpl.h>
> +
> +.align 3
> +ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm_Fence)
> +ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm_Fence)
> +
> +ASM_PFX(RiscVInvalidateInstCacheAsm_Fence):
> +    fence.i
> +    ret
> +
> +ASM_PFX(RiscVInvalidateDataCacheAsm_Fence):
> +    fence
> +    ret
> +
> +ASM_GLOBAL ASM_PFX (RiscVCpuCacheFlushAsm_Cbo)
> +ASM_PFX (RiscVCpuCacheFlushAsm_Cbo):
> +
> +  .long 0x0025200f
Can we have macros instead of magic number?

> +  ret
> +
> +ASM_GLOBAL ASM_PFX (RiscVCpuCacheCleanAsm_Cbo)
> +ASM_PFX (RiscVCpuCacheCleanAsm_Cbo):
> +  .long 0x0015200f
> +  ret
> +
> +ASM_GLOBAL ASM_PFX (RiscVCpuCacheInvalAsm_Cbo)
> +ASM_PFX (RiscVCpuCacheInvalAsm_Cbo):
> +  .long 0x0005200f
> +  ret
> +
> +ASM_GLOBAL ASM_PFX(RiscvIsCbiEnabledAsm)
> +ASM_PFX(RiscvIsCbiEnabledAsm):
> +    li    a0, 3
> +    csrr  a1, CSR_SENVCFG
> +    and   a1, a1, SENVCFG_CBIE

This is not correct. SENVCFG is only for U-mode. It can not be relied
upon in S-mode. We have to use extension discovery itself. Same comment
for CBCFE also.

Thanks,
Sunil

> +    beqz  a1, skip
> +
> +    and   a1, a1, 0x10
> +    beqz  a1, skip
> +
> +    mv    a0, x0
> +skip:
> +    ret
> +
> +ASM_GLOBAL ASM_PFX(RiscvIsCbcfeEnabledAsm)
> +ASM_PFX(RiscvIsCbcfeEnabledAsm):
> +    li    a0, 3
> +    csrr  a1, CSR_SENVCFG
> +    and   a1, a1, SENVCFG_CBCFE
> +    beqz  a1, next
> +
> +    mv    a0, x0
> +next:
> +    ret
> -- 
> 2.34.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107156): https://edk2.groups.io/g/devel/message/107156
Mute This Topic: https://groups.io/mt/100117169/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO

[Dhaval Sharma]

[-- Attachment #1: Type: text/plain, Size: 15650 bytes --]

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 <sunilvl@ventanamicro.com> wrote:

> Hi Dhaval,
>
> On Thu, Jul 13, 2023 at 03:03:31PM +0530, Dhaval wrote:
> > From: Dhaval Sharma <dhaval@rivosinc.com>
> >
> > 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 <ardb+tianocore@kernel.org>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Sunil V L <sunilvl@ventanamicro.com>
> > Cc: Andrei Warkentin <andrei.warkentin@intel.com>
> > Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> > ---
> >
> > 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.<BR>
> > +  Copyright (c) 2023, Rivos Inc. All rights reserved.<BR>
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >  **/
> > @@ -10,13 +11,21 @@
> >  #include <Library/BaseLib.h>
> >  #include <Library/DebugLib.h>
> >
> > +#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 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 mode, then Address is a virtual address.
> > +
> > +  @param  Length  The number of bytes to invalidate from the instruction
> > +          cache.
> > +
> > +  @param  Op  Type of CMO operation to be performed
> > +
> > +  @return Address.
> > +
> > +**/
> > +VOID *
> > +EFIAPI
> > +CacheOpCacheRange (
> > +  IN VOID      *Address,
> > +  IN UINTN     Length,
> > +  IN CACHE_OP  Op
> > +  )
> > +{
> > +  UINTN  CacheLineSize;
> > +  UINTN  Start;
> > +  UINTN  End;
> > +
> > +  if (Length == 0) {
> > +    return Address;
> > +  }
> > +
> > +  ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Address));
> > +
> > +  //
> > +  // Cache line size is 8 * Bits 15-08 of EBX returned from CPUID 01H
> > +  //
> This comments is invalid for RISC-V.
>
> > +  CacheLineSize = RV64_CACHE_BLOCK_SIZE;
> > +
> > +  Start = (UINTN)Address;
> > +  //
> > +  // Calculate the cache line alignment
> > +  //
> > +  End    = (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize -
> 1);
> > +  Start &= ~((UINTN)CacheLineSize - 1);
> > +
> > +  DEBUG (
> > +    (DEBUG_INFO,
> > +     "%a Performing Cache Management Operation %d \n", __func__, Op)
> > +    );
> > +
> > +  do {
> > +    switch (Op) {
> > +      case Invld:
> > +        RiscVCpuCacheInvalAsm_Cbo (Start);
> > +        break;
> > +      case Flush:
> > +        RiscVCpuCacheFlushAsm_Cbo (Start);
> > +        break;
> > +      case Clean:
> > +        RiscVCpuCacheCleanAsm_Cbo (Start);
> > +        break;
> > +      default:
> > +        DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported operation\n"));
> > +        break;
> > +    }
> > +
> > +    Start = Start + CacheLineSize;
> > +  } while (Start != End);
> > +
> > +  return Address;
> > +}
> > +
> >  /**
> >    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 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
> > @@ -41,7 +181,7 @@ InvalidateInstructionCache (
> >    VOID
> >    )
> >  {
> > -  RiscVInvalidateInstCacheAsm ();
> > +  RiscVInvalidateInstCacheAsm_Fence ();
> >  }
> >
> >  /**
> > @@ -76,12 +216,17 @@ InvalidateInstructionCacheRange (
> >    IN UINTN  Length
> >    )
> >  {
> > -  DEBUG (
> > -    (DEBUG_WARN,
> > -     "%a:RISC-V unsupported function.\n"
> > -     "Invalidating the whole instruction cache instead.\n", __func__)
> > -    );
> > -  InvalidateInstructionCache ();
> > +  if (RiscvIsCbiEnabledAsm () == RETURN_SUCCESS) {
> > +    CacheOpCacheRange (Address, Length, Invld);
> > +  } else {
> > +    DEBUG (
> > +      (DEBUG_WARN,
> > +       "%a:RISC-V unsupported function.\n"
> > +       "Invalidating the whole instruction cache instead.\n", __func__)
> > +      );
> > +    InvalidateInstructionCache ();
> > +  }
> > +
> >    return Address;
> >  }
> >
> > @@ -137,7 +282,12 @@ WriteBackInvalidateDataCacheRange (
> >    IN      UINTN  Length
> >    )
> >  {
> > -  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> > +  if (RiscvIsCbcfeEnabledAsm () == RETURN_SUCCESS) {
> > +    CacheOpCacheRange (Address, Length, Flush);
> > +  } else {
> > +    DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n",
> __func__));
> > +  }
> > +
> >    return Address;
> >  }
> >
> > @@ -192,7 +342,12 @@ WriteBackDataCacheRange (
> >    IN      UINTN  Length
> >    )
> >  {
> > -  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> > +  if (RiscvIsCbcfeEnabledAsm () == RETURN_SUCCESS) {
> > +    CacheOpCacheRange (Address, Length, Clean);
> > +  } else {
> > +    DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n",
> __func__));
> > +  }
> > +
> >    return Address;
> >  }
> >
> > @@ -213,7 +368,12 @@ InvalidateDataCache (
> >    VOID
> >    )
> >  {
> > -  RiscVInvalidateDataCacheAsm ();
> > +  DEBUG (
> > +    (DEBUG_WARN,
> > +     "%a:RISC-V unsupported function.\n"
> > +     "Invalidating the whole instruction cache instead.\n", __func__)
> Data cache?
> > +    );
> > +  RiscVInvalidateDataCacheAsm_Fence ();
> >  }
> >
> >  /**
> > @@ -250,6 +410,16 @@ InvalidateDataCacheRange (
> >    IN      UINTN  Length
> >    )
> >  {
> > -  DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__));
> > +  if (RiscvIsCbiEnabledAsm () == RETURN_SUCCESS) {
> > +    CacheOpCacheRange (Address, Length, Invld);
> > +  } else {
> > +    DEBUG (
> > +      (DEBUG_WARN,
> > +       "%a:RISC-V unsupported function.\n"
> > +       "Invalidating the whole instruction cache instead.\n", __func__)
> Data cache?
>
> > +      );
> > +    InvalidateDataCache ();
> > +  }
> > +
> >    return Address;
> >  }
> > diff --git a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> b/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> > deleted file mode 100644
> > index 7c10fdd268af..000000000000
> > --- a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> >
> -//------------------------------------------------------------------------------
> > -//
> > -// RISC-V cache operation.
> > -//
> > -// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> rights reserved.<BR>
> > -//
> > -// SPDX-License-Identifier: BSD-2-Clause-Patent
> > -//
> >
> -//------------------------------------------------------------------------------
> > -
> > -.align 3
> > -ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm)
> > -ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm)
> > -
> > -ASM_PFX(RiscVInvalidateInstCacheAsm):
> > -    fence.i
> > -    ret
> > -
> > -ASM_PFX(RiscVInvalidateDataCacheAsm):
> > -    fence
> > -    ret
> Have you done git mv first to rename the file so that git history is
> maintained?
>
> > diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> > new file mode 100644
> > index 000000000000..ecf391632221
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> > @@ -0,0 +1,64 @@
> >
> +//------------------------------------------------------------------------------
> > +//
> > +// RISC-V cache operation.
> > +//
> > +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> rights reserved.<BR>
> > +// Copyright (c) 2022, Rivos Inc. All rights reserved.<BR>
> > +//
> > +// SPDX-License-Identifier: BSD-2-Clause-Patent
> > +//
> >
> +//------------------------------------------------------------------------------
> > +#include <Register/RiscV64/RiscVImpl.h>
> > +
> > +.align 3
> > +ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm_Fence)
> > +ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm_Fence)
> > +
> > +ASM_PFX(RiscVInvalidateInstCacheAsm_Fence):
> > +    fence.i
> > +    ret
> > +
> > +ASM_PFX(RiscVInvalidateDataCacheAsm_Fence):
> > +    fence
> > +    ret
> > +
> > +ASM_GLOBAL ASM_PFX (RiscVCpuCacheFlushAsm_Cbo)
> > +ASM_PFX (RiscVCpuCacheFlushAsm_Cbo):
> > +
> > +  .long 0x0025200f
> Can we have macros instead of magic number?
>
> > +  ret
> > +
> > +ASM_GLOBAL ASM_PFX (RiscVCpuCacheCleanAsm_Cbo)
> > +ASM_PFX (RiscVCpuCacheCleanAsm_Cbo):
> > +  .long 0x0015200f
> > +  ret
> > +
> > +ASM_GLOBAL ASM_PFX (RiscVCpuCacheInvalAsm_Cbo)
> > +ASM_PFX (RiscVCpuCacheInvalAsm_Cbo):
> > +  .long 0x0005200f
> > +  ret
> > +
> > +ASM_GLOBAL ASM_PFX(RiscvIsCbiEnabledAsm)
> > +ASM_PFX(RiscvIsCbiEnabledAsm):
> > +    li    a0, 3
> > +    csrr  a1, CSR_SENVCFG
> > +    and   a1, a1, SENVCFG_CBIE
>
> This is not correct. SENVCFG is only for U-mode. It can not be relied
> upon in S-mode. We have to use extension discovery itself. Same comment
> for CBCFE also.
>
> Thanks,
> Sunil
>
> > +    beqz  a1, skip
> > +
> > +    and   a1, a1, 0x10
> > +    beqz  a1, skip
> > +
> > +    mv    a0, x0
> > +skip:
> > +    ret
> > +
> > +ASM_GLOBAL ASM_PFX(RiscvIsCbcfeEnabledAsm)
> > +ASM_PFX(RiscvIsCbcfeEnabledAsm):
> > +    li    a0, 3
> > +    csrr  a1, CSR_SENVCFG
> > +    and   a1, a1, SENVCFG_CBCFE
> > +    beqz  a1, next
> > +
> > +    mv    a0, x0
> > +next:
> > +    ret
> > --
> > 2.34.1
> >
>


-- 
Thanks!
=D


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107292): https://edk2.groups.io/g/devel/message/107292
Mute This Topic: https://groups.io/mt/100117169/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 20220 bytes --]