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

[LIU Zhiwei]

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

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