| Message ID | 20210410194047.559189-1-josemartins90@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: fix wfi exception behavior | expand |
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 > >
> > - 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? > In section 5.4, the spec states that mstatus.tw has effect regardless of virtualization mode: "The TW field affects execution in all modes except M-mode.". I interpret "all modes" as being all supervisor modes since section 3.1.6.5 states that "When S-mode is implemented, then executing WFI in U-mode causes an illegal instruction exception" and later chapter 5 says that a virtual instruction exception should be generated when "in VU-mode, attempts to execute WFI (...)" regardless of the state of any status bit. Plus, it should be an illegal instruction exception and not a virtual instruction exception even in VS-mode when mstatus.tw = 1 because the spec also states only "When VTW=1 *(and assuming mstatus.TW=0)*, an attempt in VS-mode to execute WFI raises a virtual instruction exception". But just now I realized the patch is assuming S-mode is present and not taking into account M/U only harts. If this is the case TW should affect U-mode WFIs. I will fix this and submit a new version of the patch. José
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))) { + 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;
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(-)