On 2023/3/23 06:20, Daniel Henrique Barboza wrote:
write_misa() must use as much common logic as possible. We want to open
code just the bits that are exclusive to the CSR write operation and TCG
internals.
Rewrite write_misa() to work as follows:
- mask the write using misa_ext_mask to avoid enabling unsupported
extensions;
- suppress RVC if the next insn isn't aligned;
- handle RVE. This is done by filtering all bits but RVE from 'val'.
Setting RVE will forcefully set only RVE - assuming it gets
validated afterwards;
- emulate the steps done by realize(): validate the candidate misa_ext
val, then validate the configuration with the candidate misa_ext val,
and finally commit the changes to cpu->cfg.
If any of the validation steps fails, the write operation is a no-op.
Let's keep write_misa() as experimental for now until this logic gains
enough mileage.
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
target/riscv/cpu.c | 12 ++++------
target/riscv/cpu.h | 6 +++++
target/riscv/csr.c | 59 ++++++++++++++++++++++++++--------------------
3 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0e6b8fb45e..41b17ba0c3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1018,9 +1018,8 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU
*cpu)
}
-static void riscv_cpu_validate_misa_ext(CPURISCVState *env,
- uint32_t misa_ext,
- Error **errp)
+void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,
+ Error **errp)
{
Error *local_err = NULL;
@@ -1113,9 +1112,8 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
* candidate misa_ext value. No changes in env->misa_ext
* are made.
*/
-static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
- uint32_t misa_ext,
- Error **errp)
+void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
+ Error **errp)
{
if (cpu->cfg.epmp && !cpu->cfg.pmp) {
/*
@@ -1206,7 +1204,7 @@ static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
}
}
-static void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
{
if (cpu->cfg.ext_zk) {
cpu->cfg.ext_zkn = true;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index dbb4df9df0..ca2ba6a647 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -593,6 +593,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
char *riscv_isa_string(RISCVCPU *cpu);
void riscv_cpu_list(void);
+void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,
+ Error **errp);
+void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
+ Error **errp);
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu);
+
#define cpu_list riscv_cpu_list
#define cpu_mmu_index riscv_cpu_mmu_index
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..8d5e8f9ad1 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1343,39 +1343,17 @@ static RISCVException read_misa(CPURISCVState *env, int
csrno,
static RISCVException write_misa(CPURISCVState *env, int csrno,
target_ulong val)
{
+ RISCVCPU *cpu = env_archcpu(env);
+ Error *local_err = NULL;
+
if (!riscv_cpu_cfg(env)->misa_w) {
/* drop write to misa */
return RISCV_EXCP_NONE;
}
- /* 'I' or 'E' must be present */
- if (!(val & (RVI | RVE))) {
- /* It is not, drop write to misa */
- return RISCV_EXCP_NONE;
- }
-
- /* 'E' excludes all other extensions */
- if (val & RVE) {
- /*
- * when we support 'E' we can do "val = RVE;" however
- * for now we just drop writes if 'E' is present.
- */
- return RISCV_EXCP_NONE;
- }
-
- /*
- * misa.MXL writes are not supported by QEMU.
- * Drop writes to those bits.
- */
-
/* Mask extensions that are not supported by this hart */
val &= env->misa_ext_mask;
- /* 'D' depends on 'F', so clear 'D' if 'F' is not present */
- if ((val & RVD) && !(val & RVF)) {
- val &= ~RVD;
- }
-
/*
* Suppress 'C' if next instruction is not aligned
* TODO: this should check next_pc
@@ -1389,6 +1367,37 @@ static RISCVException write_misa(CPURISCVState *env, int
csrno,
return RISCV_EXCP_NONE;
}
+ /*
+ * We'll handle special cases in separate. If one
+ * of these bits are enabled we'll handle them and
+ * end the CSR write.
+ */
+ if (val & RVE && !(env->misa_ext & RVE)) {
+ /*
+ * RVE must be enabled by itself. Clear all other
+ * misa_env bits and let the validation do its
+ * job.
+ */
+ val &= RVE;
It can just be val = RVE here.
Regards,
Weiwei Li
+ }
+
+ /*
+ * This flow is similar to what riscv_cpu_realize() does,
+ * with the difference that we will update env->misa_ext
+ * value if everything is ok.
+ */
+ riscv_cpu_validate_misa_ext(env, val, &local_err);
+ if (local_err != NULL) {
+ return RISCV_EXCP_NONE;
+ }
+
+ riscv_cpu_validate_extensions(cpu, val, &local_err);
+ if (local_err != NULL) {
+ return RISCV_EXCP_NONE;
+ }
+
+ riscv_cpu_commit_cpu_cfg(cpu);
+
if (!(val & RVF)) {
env->mstatus &= ~MSTATUS_FS;
}