On 2023/2/16 02:57, 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.
Regardless, the spec says that MISA is a WARL read-write CSR, and gating
the writes in the register doesn't make sense. OS and applications
should be wary of the consequences when writing it, but the write itself
must always be allowed.
Remove the RISCV_FEATURE_MISA verification at the start of write_misa(),
removing RISCV_FEATURE_MISA altogether since there will be no more
callers of this enum.
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
target/riscv/cpu.h | 1 -
target/riscv/csr.c | 5 -----
2 files changed, 6 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7128438d8e..01803a020d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -89,7 +89,6 @@ enum {
RISCV_FEATURE_MMU,
RISCV_FEATURE_PMP,
RISCV_FEATURE_EPMP,
- RISCV_FEATURE_MISA,
RISCV_FEATURE_DEBUG
};
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e149b453da..5bd4cdbef5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1329,11 +1329,6 @@ 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)) {
- /* drop write to misa */
- return RISCV_EXCP_NONE;
- }
-
I have a question here:
If we directly remove this here without other limits, the bugs
introduced by following code
will be exposed to users, such as:
- V may be still enabled when misa.D is cleared.
- Zfh/Zfhmin may be still enabled when misa.D is cleared
- Misa.U may be cleared when Misa.S is still set.
...
Should we fix these bugs before this patch? Or fix them in following
patchset?
If we choose the latter, I think it's better to add a limitation(such
as writable_mask) to the changable fields of misa here.
Regards,
Weiwei Li
/* 'I' or 'E' must be present */
if (!(val & (RVI | RVE))) {
/* It is not, drop write to misa */
