[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
>
>