On Wed, Feb 15, 2023 at 3:26 AM Daniel Henrique Barboza <dbarb...@ventanamicro.com> 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.
I am surprised to see we have a RISCV_FEATURE_MISA. Per spec MISA is a WARL read-write CSR. I don't think creating a special config property for a read-write CSR makes sense. We should drop the feature enum and the feature check in write_misa() directly. > > Reviewed-by: Weiwei Li <liwei...@iscas.ac.cn> > Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> > --- > target/riscv/cpu.c | 1 + > target/riscv/cpu.h | 1 + > target/riscv/csr.c | 2 +- > 3 files changed, 3 insertions(+), 1 deletion(-) > Regards, Bin
