On Fri, 23 Aug 2019 16:38:00 PDT (-0700), Alistair Francis wrote:
Update the CSR permission checking to work correctly when we are in
HS-mode.
Signed-off-by: Alistair Francis
---
target/riscv/csr.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f767ad24be..471f23a1d0 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -799,9 +799,15 @@ int riscv_csrrw(CPURISCVState *env, int csrno,
target_ulong *ret_value,
/* check privileges and return -1 if check fails */
#if !defined(CONFIG_USER_ONLY)
-int csr_priv = get_field(csrno, 0x300);
+int csr_priv = env->priv;
This isn't really "csr_priv" (ie, the priv needed to access the CSR) any more,
it's really the effective priv of the machine. Leaving the variable with the
same name makes this hard to read, but I think it is correct.
int read_only = get_field(csrno, 0xC00) == 3;
-if ((write_mask && read_only) || (env->priv < csr_priv)) {
+
+if (riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
+/* Plus 1 as we are in HS mode */
The comment is useless, it doesn't say why we increment it. Also, I don't
think this is correct: doesn't it allow U mode to access S CSRs when H is
present and V is disabled?
Something like
riscv_effective_priv(CPURISCVState *env)
{
if (riscv_has_ext(env, RVH) && env->priv == PRIV_S &&
!riscv_cpu_virt_enabled(env)) {
return PRIV_HS;
}
return env->priv;
}
would probably be used in a handful of places, and would be a drop in for
env->priv here.
+csr_priv++;
+}
+
+if ((write_mask && read_only) || (csr_priv < get_field(csrno, 0x300))) {
return -1;
}
#endif
