[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/riscv/csr.c: fix H extension TVM trap
From: |
Daniel Henrique Barboza |
Subject: |
Re: [PATCH] target/riscv/csr.c: fix H extension TVM trap |
Date: |
Wed, 8 Mar 2023 16:44:03 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 |
On 3/8/23 09:34, chenyi2000@zju.edu.cn wrote:
From: Yi Chen <chenyi2000@zju.edu.cn>
Trap accesses to hgatp if MSTATUS_TVM is enabled.
Don't trap accesses to vsatp even if MSTATUS_TVM is enabled.
Signed-off-by: Yi Chen <chenyi2000@zju.edu.cn>
---
target/riscv/csr.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab56663..09bc780 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2655,7 +2655,7 @@ static RISCVException read_satp(CPURISCVState *env, int
csrno,
return RISCV_EXCP_NONE;
}
- if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+ if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
get_field(env->mstatus, MSTATUS_TVM)) {
The commit message mentions 'vsatp' but this patch is changing satp callbacks.
Any reason to not change read_vsatp() and write_vsatp() instead?
return RISCV_EXCP_ILLEGAL_INST;
} else {
*val = env->satp;
@@ -2683,7 +2683,7 @@ static RISCVException write_satp(CPURISCVState *env, int
csrno,
}
if (vm && mask) {
- if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+ if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env) &&
get_field(env->mstatus, MSTATUS_TVM)) {
return RISCV_EXCP_ILLEGAL_INST;
} else {
/*
@@ -3047,14 +3047,24 @@ static RISCVException read_hgeip(CPURISCVState *env,
int csrno,
static RISCVException read_hgatp(CPURISCVState *env, int csrno,
target_ulong *val)
{
- *val = env->hgatp;
+ if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+ return RISCV_EXCP_ILLEGAL_INST;
The end of the first paragraph of ISA 8.2.10 goes as follows:
====
When mstatus.TVM=1, attempts to read or write hgatp while executing
in HS-mode will raise an illegal instruction exception.
====
I believe you need to check for HS-mode, not just PRV_S. riscv_csrrw_check() in
target/riscv/csr.c checks for HS-mode as follows:
if (riscv_has_ext(env, RVH) && env->priv == PRV_S &&
!riscv_cpu_virt_enabled(env)) {
Same goes for write_hgatp() below.
+ } else {
+ *val = env->hgatp;
+ }
+
You can discard the 'else' since you're doing a return in the if:
if (...) {
return RISCV_EXCP_ILLEGAL_INST;
}
*val = env->hgatp;
return RISCV_EXCP_NONE;
}
static RISCVException write_hgatp(CPURISCVState *env, int csrno,
target_ulong val)
{
- env->hgatp = val;
+ if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ } else {
+ env->hgatp = val;
+ }
No need for else here either:
if (...) {
return RISCV_EXCP_ILLEGAL_INST;
}
env->hgatp = val;
Thanks,
Daniel
+
return RISCV_EXCP_NONE;
}