On 2023/3/19 04:04, Daniel Henrique Barboza wrote:
Allow write_misa() to enable RVV like we did with RVG. We'll need a
riscv_cpu_enable_v() to enable all related misa bits and Z extensions.
This new helper validates the existing 'env' conf by using the existing
riscv_cpu_validate_v(). We'll also check if we'll be able to enable 'F'
by checking for ext_zfinx.
As with RVG, enabling RVV is considered to be a standalone operation in
write_misa(). This means that we'll guarantee that we're not being
inconsistent in riscv_cpu_enable_v() and that we're okay with skipping
regular validation.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/cpu.c | 35 +++++++++++++++++++++++++++++++++++
target/riscv/cpu.h | 1 +
target/riscv/csr.c | 14 ++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 73a5fa46ee..9c16b29f27 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -983,6 +983,41 @@ static void riscv_cpu_validate_v(CPURISCVState *env,
env->vext_ver = vext_version;
}
+target_ulong riscv_cpu_enable_v(RISCVCPU *cpu, Error **errp)
+{
+ CPURISCVState *env = &cpu->env;
+ RISCVCPUConfig *cfg = &cpu->cfg;
+ Error *local_err = NULL;
+
+ riscv_cpu_validate_v(env, cfg, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return 0;
+ }
This check is not necessary, we call this function only when we enable v by
write_misa, which also have a prerequisite:
V is enabled at the very first. So this check will always be true, since the
parameter for vector cannot be changed dynamically.
Similar to following check.
+
+ if (cpu->cfg.ext_zfinx) {
+ error_setg(errp, "Unable to enable V: Zfinx is enabled, "
+ "so F can not be enabled");
+ return 0;
+ }
+
+ cfg->ext_f = true;
+ env->misa_ext |= RVF;
+
+ cfg->ext_d = true;
+ env->misa_ext |= RVD;
We do check V against F/D at first. Why we do this when enable V?
And if we do this, whether we should also enable F when enable D?
+
+ /*
+ * The V vector extension depends on the
+ * Zve32f, Zve64f and Zve64d extensions.
+ */
+ cpu->cfg.ext_zve64d = true;
+ cpu->cfg.ext_zve64f = true;
+ cpu->cfg.ext_zve32f = true;
This is right, but not necessary in current implementation, since they will not
be disabled when we disable V.
So we needn't enable them when we re-enable V.
+
+ return env->misa_ext;
+}
+
static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp)
{
CPURISCVState *env = &cpu->env;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3ca1d4903c..45e801d926 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -600,6 +600,7 @@ void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t
misa_ext,
void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu, uint32_t misa_ext);
target_ulong riscv_cpu_enable_g(RISCVCPU *cpu, Error **errp);
+target_ulong riscv_cpu_enable_v(RISCVCPU *cpu, Error **errp);
#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 4335398c19..e9e1afc57e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1395,6 +1395,20 @@ static RISCVException write_misa(CPURISCVState *env, int
csrno,
goto commit;
}
+ if (val & RVV && !(env->misa_ext & RVV)) {
+ /*
+ * If the write wants to enable RVV, only RVV and
+ * its dependencies will be updated in the CSR.
+ */
+ val = riscv_cpu_enable_v(cpu, &local_err);
+ if (local_err != NULL) {
+ return RISCV_EXCP_NONE;
+ }
+
+ val |= RVV;
+ goto commit;
+ }
+
So, I think we can just treat V as common extension, and do nothing
additionally for disabling/re-enabling it.