SSTC exception behavior

[JJ-Gaisler]

Hi, I am having issues with the exception behavior of read/writes of the stimecmp/vstimecmp CSRs. The priv spec clearly states that m/hcounteren.tm has relevance for the exception behavior of these CSRs.

"In addition, when the TM bit in the mcounteren register is clear, attempts to access the stimecmp or vstimecmp register while executing in a mode less privileged than M will cause an illegal instruction exception."

I checked the implementation of the stimecmp_csr_t class and this one doesn't consider the counteren CSRs at all. Below you can find a patch for what I think is the correct behavior, for now keeping the debug prints in there.

I have a test which loops through all state which can affect the exception behavior of the sstc extension, i.e. misa.h m/hcounteren, m/henvcfg, privilege mode, virtualization mode. If the test runs successfully it should only fail on an assert for current_checksum. The test can be ran with: spike -m0x80000000:0x10000000,0x20000000:0x1000,0x4000:0x1000,0xB000:0x1000 --isa rv64gvh_svinval_zba_zbb_zbc_zbs_zfh_zbkb_zicsr_zifencei_sscofpmf_smepmp_zicbom_zicntr_zihpm_svadu_sstc -p1 hypervisor_tests.txt hypervisor_tests.txt

diff --git a/riscv/csrs.cc b/riscv/csrs.cc
index 8d7737f1..4c7bffd7 100644
--- a/riscv/csrs.cc
+++ b/riscv/csrs.cc
@@ -1,4 +1,5 @@
 // See LICENSE for license details.
+// clang-format off

 // For std::any_of
 #include <algorithm>
@@ -15,6 +16,7 @@
 #include "insn_macros.h"
 // For CSR_DCSR_V:
 #include "debug_defines.h"
+#include <format>

 // STATE macro used by require_privilege() macro:
 #undef STATE
@@ -1571,7 +1573,10 @@ virtualized_stimecmp_csr_t::virtualized_stimecmp_csr_t(processor_t* const proc,
 }

 void stimecmp_csr_t::verify_permissions(insn_t insn, bool write) const {
-  if (!(state->menvcfg->read() & MENVCFG_STCE)) {
+  constexpr reg_t TM = 0b10;
+  std::cerr << std::format("menvcfg.stce = {:#x} mcounteren.tm = {:#x}\n",
+                           state->menvcfg->read() & MENVCFG_STCE, state->mcounteren->read() & TM);
+  if (!((state->menvcfg->read() & MENVCFG_STCE) or !(state->mcounteren->read() & TM))) {
     // access to (v)stimecmp with MENVCFG.STCE = 0
     if (state->prv < PRV_M)
       throw trap_illegal_instruction(insn.bits());
@@ -1579,7 +1584,9 @@ void stimecmp_csr_t::verify_permissions(insn_t insn, bool write) const {

   state->time_proxy->verify_permissions(insn, false);

-  if (state->v && !(state->henvcfg->read() & HENVCFG_STCE)) {
+  if (state->v && (!(state->henvcfg->read() & HENVCFG_STCE) or !(state->hcounteren->read() & TM))) {
+    std::cerr << std::format("henvcfg.stce = {:#x} hcounteren.tm = {:#x}\n",
+                           state->henvcfg->read() & HENVCFG_STCE, state->hcounteren->read() & TM);
     // access to vstimecmp with MENVCFG.STCE = 1 and HENVCFG.STCE = 0 when V = 1
     throw trap_virtual_instruction(insn.bits());
   }

[aswaterman]

cc @i2h2, who contributed Sstc extension support

[i2h2]

I believe xcounteren is checked by state->time_proxy->verify_permissions. Please let me know if that's not the case. The spec says the privilege mode needs to be S or VS, e.g., "when the TM bit in the mcounteren register is clear, attempts to access the stimecmp register while executing in S-mode will cause an illegal instruction exception."

[JJ-Gaisler]

Tanks for the clarification, it looks like my test had a bug, closing the issue.