qemu-devel
[Top][All Lists]
Advanced

[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;
  }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]