Re: Re: [PATCH] target/riscv/csr.c: fix H extension TVM trap

["CHEN Yi" <chenyi2000@zju.edu.cn>]

[-- Attachment #1: Type: text/plain, Size: 4139 bytes --]




-----Original Messages-----
From:"LIU Zhiwei" <zhiwei_liu@linux.alibaba.com>
Sent Time:2023-03-10 17:18:56 (Friday)
To: "CHEN Yi" <chenyi2000@zju.edu.cn>, qemu-devel@nongnu.org
Cc: "Palmer Dabbelt" <palmer@dabbelt.com>, "Alistair Francis" <alistair.francis@wdc.com>, "Bin Meng" <bin.meng@windriver.com>, "Weiwei Li" <liweiwei@iscas.ac.cn>, "Daniel Henrique Barboza" <dbarboza@ventanamicro.com>, "open list:RISC-V TCG CPUs" <qemu-riscv@nongnu.org>
Subject: Re: [PATCH] target/riscv/csr.c: fix H extension TVM trap






On 2023/3/10 17:08, CHEN Yi wrote:

-----Original Messages-----
From:"LIU Zhiwei" <zhiwei_liu@linux.alibaba.com>
Sent Time:2023-03-10 10:12:10 (Friday)
To:chenyi2000@zju.edu.cn, qemu-devel@nongnu.org
Cc: "Palmer Dabbelt" <palmer@dabbelt.com>, "Alistair Francis" <alistair.francis@wdc.com>, "Bin Meng" <bin.meng@windriver.com>, "Weiwei Li" <liweiwei@iscas.ac.cn>, "Daniel Henrique Barboza" <dbarboza@ventanamicro.com>, "open list:RISC-V TCG CPUs" <qemu-riscv@nongnu.org>
Subject: Re: [PATCH] target/riscv/csr.c: fix H extension TVM trap






On 2023/3/8 20: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.

By the way, do you know why mstatus_tvm and hstatus_tvm are needed?

The specification said,

The TVM mechanism improves virtualization efficiency by permitting guest operating systems to
execute in S-mode, rather than classically virtualizing them in U-mode. This approach obviates
the need to trap accesses to most S-mode CSRs.


I don't know how the tvm field obviates the need to trap accesses to most S-mode CSRs.

Thanks,
Zhiwei




When VMs are in U-mode, their accesses to S-mode CSR registers must be emulated. Otherwise, the behavior of the host OS will be affected. But I guess since TVM helps insert another stage of address translation below that controlled by the OS, it enables VMs to run in S-mode, which means that VMs can directly use most S-mode CSR registers instead of emulated ones.

If the guest running in S-mode, the other smode CSR accesses may break the host OS. 

Zhiwei




I guess hypervisors can be (partially) put in M-mode. Do you have any example where access to a specific CSR has to be trapped?




Best,

Yi




Best,

Yi







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)) {
         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;
+    } else {
+        *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;
+    }
+
     return RISCV_EXCP_NONE;
 } 

[-- Attachment #2: Type: text/html, Size: 7478 bytes --]