-----Original Messages----- From:"LIU Zhiwei" <zhiwei_...@linux.alibaba.com> Sent Time:2023-03-10 10:12:10 (Friday) To: chenyi2...@zju.edu.cn, qemu-devel@nongnu.org Cc: "Palmer Dabbelt" <pal...@dabbelt.com>, "Alistair Francis" <alistair.fran...@wdc.com>, "Bin Meng" <bin.m...@windriver.com>, "Weiwei Li" <liwei...@iscas.ac.cn>, "Daniel Henrique Barboza" <dbarb...@ventanamicro.com>, "open list:RISC-V TCG CPUs" <qemu-ri...@nongnu.org> Subject: Re: [PATCH] target/riscv/csr.c: fix H extension TVM trap
On 2023/3/8 20:34, chenyi2...@zju.edu.cn wrote: From: Yi Chen <chenyi2...@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. Best, Yi Signed-off-by: Yi Chen <chenyi2...@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; }