On 2023/2/10 21:36, Daniel Henrique Barboza wrote:
At this moment, and apparently since ever, we have no way of enabling
RISCV_FEATURE_MISA. This means that all the code from write_misa(), all
the nuts and bolts that handles how to properly write this CSR, has
always been a no-op as well because write_misa() will always exit
earlier.
This seems to be benign in the majority of cases. Booting an Ubuntu
'virt' guest and logging all the calls to 'write_misa' shows that no
writes to MISA CSR was attempted. Writing MISA, i.e. enabling/disabling
RISC-V extensions after the machine is powered on, seems to be a niche
use.
There is a good chance that the code in write_misa() hasn't been
properly tested. Allowing users to write MISA can open the floodgates of
new breeds of bugs. We could instead remove most (if not all) of
write_misa() since it's never used. Well, as a hardware emulator,
dealing with crashes because a register write went wrong is what we're
here for.
Create a 'misa-w' CPU property to allow users to choose whether writes
to MISA should be allowed. The default is set to 'false' for all RISC-V
machines to keep compatibility with what we´ve been doing so far.
Read cpu->cfg.misa_w directly in write_misa(), instead of executing
riscv_set_feature(RISCV_FEATURE_MISA) in riscv_cpu_realize(), that would
simply reflect the cpu->cfg.misa_w bool value in 'env->features' and
require a riscv_feature() call to read it back.
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 1 +
target/riscv/csr.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 93b52b826c..69fb9e123f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1197,6 +1197,7 @@ static void register_cpu_props(DeviceState *dev)
static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+ DEFINE_PROP_BOOL("misa-w", RISCVCPU, cfg.misa_w, false),
DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, RISCV_CPU_MARCHID),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7128438d8e..103963b386 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -498,6 +498,7 @@ struct RISCVCPUConfig {
bool pmp;
bool epmp;
bool debug;
+ bool misa_w;
bool short_isa_string;
};
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e149b453da..4f9cc501b2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1329,7 +1329,9 @@ static RISCVException read_misa(CPURISCVState *env, int
csrno,
static RISCVException write_misa(CPURISCVState *env, int csrno,
target_ulong val)
{
- if (!riscv_feature(env, RISCV_FEATURE_MISA)) {
+ RISCVCPU *cpu = env_archcpu(env);
+
+ if (!cpu->cfg.misa_w) {
It's Ok to get it directly from cfg. However, personally, I prefer to
keep the non-isa features in a separate list.
Regards,
Weiwei Li
/* drop write to misa */
return RISCV_EXCP_NONE;
}
