Re: [PATCH v2] target/riscv: fix wfi exception behavior

[Alistair Francis]

On Wed, Apr 21, 2021 at 7:37 AM Jose Martins <josemartins90@gmail.com> wrote:
>
> The wfi exception trigger behavior should take into account user mode,
> hstatus.vtw, and the fact the an wfi might raise different types of
> exceptions depending on various factors:
>
> If supervisor mode is not present:
>
> - an illegal instruction exception should be generated if user mode
> executes and wfi instruction and mstatus.tw = 1.
>
> If supervisor mode is present:
>
> - when a wfi instruction is executed, an illegal exception should be triggered
> if either the current mode is user or the mode is supervisor and mstatus.tw is
> set.
>
> Plus, if the hypervisor extensions are enabled:
>
> - a virtual instruction exception should be raised when a wfi is executed from
> virtual-user or virtual-supervisor and hstatus.vtw is set.
>
> Signed-off-by: Jose Martins <josemartins90@gmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> Alistair, I hope you've agreed with my argumentis for the previous version
> of the patch. As promised, I submit this version which takes into account M/U
> only harts. It checks for the presence of the RVS extension. If it is
> not present mstatus.tw takes effect over the exection of wfi in user
> mode.
>
>  target/riscv/cpu_bits.h  |  1 +
>  target/riscv/op_helper.c | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 24b24c69c5..ed8b97c788 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -436,6 +436,7 @@
>  #define HSTATUS_HU           0x00000200
>  #define HSTATUS_VGEIN        0x0003F000
>  #define HSTATUS_VTVM         0x00100000
> +#define HSTATUS_VTW          0x00200000
>  #define HSTATUS_VTSR         0x00400000
>  #if defined(TARGET_RISCV64)
>  #define HSTATUS_VSXL        0x300000000
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d55def76cf..15982a7a33 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -173,10 +173,15 @@ target_ulong helper_mret(CPURISCVState *env, 
> target_ulong cpu_pc_deb)
>  void helper_wfi(CPURISCVState *env)
>  {
>      CPUState *cs = env_cpu(env);
> +    bool rvs = riscv_has_ext(env, RVS);
> +    bool prv_u = env->priv == PRV_U;
> +    bool prv_s = env->priv == PRV_S;
>
> -    if ((env->priv == PRV_S &&
> -        get_field(env->mstatus, MSTATUS_TW)) ||
> -        riscv_cpu_virt_enabled(env)) {
> +    if (((prv_s || (!rvs && prv_u)) && get_field(env->mstatus, MSTATUS_TW)) 
> ||
> +        (rvs && prv_u && !riscv_cpu_virt_enabled(env))) {
> +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> +    } else if (riscv_cpu_virt_enabled(env) && (prv_u ||
> +        (prv_s && get_field(env->hstatus, HSTATUS_VTW)))) {
>          riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, 
> GETPC());
>      } else {
>          cs->halted = 1;
> --
> 2.25.1
>
>