qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Alistair Francis
Subject: Re: [PATCH] target/riscv: fix wfi exception behavior
Date: Fri, 16 Apr 2021 14:55:00 +1000

On Sun, Apr 11, 2021 at 5:41 AM Jose Martins <josemartins90@gmail.com> wrote:
>
> The wfi exception trigger behavior was not taking into account the fact
> that user mode is not allowed to execute wfi instructions or the effect
> of the hstatus.vtw bit. It was also always generating virtual instruction
> exceptions when this should only happen when the wfi instruction is
> executed when the virtualization mode is enabled:
>
> - 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.
>
> - 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>
> ---
>  target/riscv/cpu_bits.h  | 1 +
>  target/riscv/op_helper.c | 8 +++++---
>  2 files changed, 6 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..354e39ec26 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -174,9 +174,11 @@ void helper_wfi(CPURISCVState *env)
>  {
>      CPUState *cs = env_cpu(env);
>
> -    if ((env->priv == PRV_S &&
> -        get_field(env->mstatus, MSTATUS_TW)) ||
> -        riscv_cpu_virt_enabled(env)) {
> +    if ((!riscv_cpu_virt_enabled(env) && env->priv == PRV_U) ||

> +        (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TW))) {

Shouldn't we check here that we aren't virtualised?

Alistair

> +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> +    } else if (riscv_cpu_virt_enabled(env) && (env->priv == PRV_U ||
> +            (env->priv == 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
>
>



reply via email to

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