Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
This commit adds support for [m|s|vs]ctrcontrol, sctrstatus and
sctrdepth CSRs handling.
Signed-off-by: Rajnesh Kanwal<rkan...@rivosinc.com>
---
target/riscv/cpu.h | 5 ++
target/riscv/cpu_cfg.h | 2 +
target/riscv/csr.c | 159 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a185e2d494..3d4d5172b8 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -263,6 +263,11 @@ struct CPUArchState {
target_ulong mcause;
target_ulong mtval; /* since: priv-1.10.0 */
+ uint64_t mctrctl;
+ uint32_t sctrdepth;
+ uint32_t sctrstatus;
+ uint64_t vsctrctl;
+
/* Machine and Supervisor interrupt priorities */
uint8_t miprio[64];
uint8_t siprio[64];
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index d9354dc80a..d329a65811 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -123,6 +123,8 @@ struct RISCVCPUConfig {
bool ext_zvfhmin;
bool ext_smaia;
bool ext_ssaia;
+ bool ext_smctr;
+ bool ext_ssctr;
bool ext_sscofpmf;
bool ext_smepmp;
bool rvv_ta_all_1s;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2f92e4b717..888084d8e5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -621,6 +621,61 @@ static RISCVException pointer_masking(CPURISCVState *env,
int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+/*
+ * M-mode:
+ * Without ext_smctr raise illegal inst excep.
+ * Otherwise everything is accessible to m-mode.
+ *
+ * S-mode:
+ * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
+ * Otherwise everything other than mctrctl is accessible.
+ *
+ * VS-mode:
+ * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
+ * Without hstateen.ctr raise virtual illegal inst excep.
+ * Otherwise allow vsctrctl, sctrstatus, 0x200-0x2ff entry range.
+ * Always raise illegal instruction exception for sctrdepth.
+ */
+static RISCVException ctr_mmode(CPURISCVState *env, int csrno)
+{
+ /* Check if smctr-ext is present */
+ if (riscv_cpu_cfg(env)->ext_smctr) {
+ return RISCV_EXCP_NONE;
+ }
+
+ return RISCV_EXCP_ILLEGAL_INST;
+}
+
+static RISCVException ctr_smode(CPURISCVState *env, int csrno)
+{
+ if ((env->priv == PRV_M && riscv_cpu_cfg(env)->ext_smctr) ||
+ (env->priv == PRV_S && !env->virt_enabled &&
+ riscv_cpu_cfg(env)->ext_ssctr)) {
+ return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
+ }
+
+ if (env->priv == PRV_S && env->virt_enabled &&
+ riscv_cpu_cfg(env)->ext_ssctr) {
+ if (csrno == CSR_SCTRSTATUS) {
missing sctrctl?
+ return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
+ }
+
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+
+ return RISCV_EXCP_ILLEGAL_INST;
+}
I think there is no need to bind M-mode with ext_smctr, S-mode with
ext_ssctr and VS-mode with ext_ssctr, since this predicate function is
for S-mode CSRs, which are defined in both smctr and ssctr, we just need
to check at least one of ext_ssctr or ext_smctr is true.
The spec states that:
Attempts to access sctrdepth from VS-mode or VU-mode raise a
virtual-instruction exception, unless CTR state enable access
restrictions apply.
In my understanding, we should check the presence of smstateen extension
first, and
if smstateen is implemented:
* for sctrctl and sctrstatus, call smstateen_acc_ok()
* for sctrdepth, call smstateen_acc_ok(), and if there is any
exception returned, always report virtual-instruction exception.
If smstateen is not implemented:
* for sctrctl and sctrstatus, there is no check.
* for sctrdepth, I think the spec is ambiguous. What does "CTR state
enable access restrictions apply" mean when smstateen is not
implemented?
Here is the code to better understand my description.
static RISCVException ctr_smode(CPURISCVState *env, int csrno)
{
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
if (!cfg->ext_ssctr && !cfg->ext_smctr) {
return RISCV_EXCP_ILLEGAL_INST;
}
if (riscv_cpu_cfg(env)->ext_smstateen) {
RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
if (ret != RISCV_EXCP_NONE) {
if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
return ret;
}
} else {
/* The spec is ambiguous. */
if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
}
return RISCV_EXCP_NONE;
}
+
+static RISCVException ctr_vsmode(CPURISCVState *env, int csrno)
+{
+ if (env->priv == PRV_S && env->virt_enabled &&
+ riscv_cpu_cfg(env)->ext_ssctr) {
+ return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
In riscv_csrrw_check(), an virtual-instruction exception is always
reported no matter what. Do we need this check?
+ }
+
+ return ctr_smode(env, csrno);
+}
+
static RISCVException aia_hmode(CPURISCVState *env, int csrno)
{
int ret;
@@ -3835,6 +3890,100 @@ static RISCVException write_satp(CPURISCVState *env,
int csrno,
return RISCV_EXCP_NONE;
}
+static RISCVException rmw_sctrdepth(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & SCTRDEPTH_MASK;
+
+ if (ret_val) {
+ *ret_val = env->sctrdepth & SCTRDEPTH_MASK;
We don't need to do bitwise and with SCTRDEPTH_MASK on read accesses
when we always do bitwise and with SCTRDEPTH_MASK on write accesses.
+ }
+
+ env->sctrdepth = (env->sctrdepth & ~mask) | (new_val & mask);
+
+ /* Correct depth. */
+ if (wr_mask & SCTRDEPTH_MASK) {
+ uint64_t depth = get_field(env->sctrdepth, SCTRDEPTH_MASK);
+
+ if (depth > SCTRDEPTH_MAX) {
+ env->sctrdepth =
+ set_field(env->sctrdepth, SCTRDEPTH_MASK, SCTRDEPTH_MAX);
+ }
+
+ /* Update sctrstatus.WRPTR with a legal value */
+ depth = 16 << depth;
The "depth" on the right side may exceed SCTRDEPTH_MAX.
+ env->sctrstatus =
+ env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
+ }
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_mctrctl(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & MCTRCTL_MASK;
+
+ if (ret_val) {
+ *ret_val = env->mctrctl & MCTRCTL_MASK;
There is no need to do bitwise and with the mask on read accesses when
we always do bitwise and with the mask on write accesses.
+ }
+
+ env->mctrctl = (env->mctrctl & ~mask) | (new_val & mask);
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_sctrctl(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & SCTRCTL_MASK;
+ RISCVException ret;
+
+ ret = rmw_mctrctl(env, csrno, ret_val, new_val, mask);
When V=1, vsctrctl substitutes for sctrctl.
+ if (ret_val) {
+ *ret_val &= SCTRCTL_MASK;
+ }
+
+ return ret;
+}
+
+static RISCVException rmw_sctrstatus(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong
wr_mask)
+{
+ uint32_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
+ uint32_t mask = wr_mask & SCTRSTATUS_MASK;
+
+ if (ret_val) {
+ *ret_val = env->sctrstatus & SCTRSTATUS_MASK;
There is no need to do bitwise and with the mask on read accesses when
we always do bitwise and with the mask on write accesses.
+ }
+
+ env->sctrstatus = (env->sctrstatus & ~mask) | (new_val & mask);
+
+ /* Update sctrstatus.WRPTR with a legal value */
+ env->sctrstatus = env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_vsctrctl(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & VSCTRCTL_MASK;
+
+ if (ret_val) {
+ *ret_val = env->vsctrctl & VSCTRCTL_MASK;
There is no need to do bitwise and with the mask on read accesses when
we always do bitwise and with the mask on write accesses.
+ }
+
+ env->vsctrctl = (env->vsctrctl & ~mask) | (new_val & mask);
+
+ return RISCV_EXCP_NONE;
+}
Is it possible to define rmw_xctrctl() instead of three individual rmw
functions and use a switch case to select the mask and the CSR for the
purpose of reducing code size?
+
static RISCVException read_vstopi(CPURISCVState *env, int csrno,
target_ulong *val)
{
@@ -5771,6 +5920,16 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_SPMBASE] = { "spmbase", pointer_masking, read_spmbase,
write_spmbase },
+ [CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL,
+ rmw_mctrctl },
I think this can be one line.
+ [CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL,
+ rmw_sctrctl },
same here
+ [CSR_SCTRDEPTH] = { "sctrdepth", ctr_smode, NULL, NULL,
+ rmw_sctrdepth },
same here
+ [CSR_SCTRSTATUS] = { "sctrstatus", ctr_smode, NULL, NULL,
+ rmw_sctrstatus },
same here
+ [CSR_VSCTRCTL] = { "vsctrctl", ctr_vsmode, NULL, NULL,
+ rmw_vsctrctl },
same here
/* Performance Counters */
[CSR_HPMCOUNTER3] = { "hpmcounter3", ctr, read_hpmcounter },
[CSR_HPMCOUNTER4] = { "hpmcounter4", ctr, read_hpmcounter },
