[PATCH] riscv: Fix bug in setting xPIE of CSR for MRET and SRET instructions

Ian Jiang posted 1 patch 4 years, 2 months ago
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200121100732.28734-1-ianjiang.ict@gmail.com
Maintainers: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>
target/riscv/op_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] riscv: Fix bug in setting xPIE of CSR for MRET and SRET instructions
Posted by Ian Jiang 4 years, 2 months ago
According to the RISC-V specification, when executing an MRET or SRET
instruction, xPIE in mstatus or sstatus should be set to 1. The orginal
QEMU does not give the right operations.
This patch fix the problem.

Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>
---
 target/riscv/op_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 331cc36232..e87c9115bc 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -93,7 +93,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
         env->priv_ver >= PRIV_VERSION_1_10_0 ?
         MSTATUS_SIE : MSTATUS_UIE << prev_priv,
         get_field(mstatus, MSTATUS_SPIE));
-    mstatus = set_field(mstatus, MSTATUS_SPIE, 0);
+    mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
     mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
     riscv_cpu_set_mode(env, prev_priv);
     env->mstatus = mstatus;
@@ -118,7 +118,7 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
         env->priv_ver >= PRIV_VERSION_1_10_0 ?
         MSTATUS_MIE : MSTATUS_UIE << prev_priv,
         get_field(mstatus, MSTATUS_MPIE));
-    mstatus = set_field(mstatus, MSTATUS_MPIE, 0);
+    mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
     mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
     riscv_cpu_set_mode(env, prev_priv);
     env->mstatus = mstatus;
-- 
2.17.1


Re: [PATCH] riscv: Fix bug in setting xPIE of CSR for MRET and SRET instructions
Posted by Alistair Francis 4 years, 2 months ago
On Tue, Jan 21, 2020 at 8:08 PM Ian Jiang <ianjiang.ict@gmail.com> wrote:
>
> According to the RISC-V specification, when executing an MRET or SRET
> instruction, xPIE in mstatus or sstatus should be set to 1. The orginal
> QEMU does not give the right operations.
> This patch fix the problem.
>
> Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/op_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 331cc36232..e87c9115bc 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -93,7 +93,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
>          env->priv_ver >= PRIV_VERSION_1_10_0 ?
>          MSTATUS_SIE : MSTATUS_UIE << prev_priv,
>          get_field(mstatus, MSTATUS_SPIE));
> -    mstatus = set_field(mstatus, MSTATUS_SPIE, 0);
> +    mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
>      mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
>      riscv_cpu_set_mode(env, prev_priv);
>      env->mstatus = mstatus;
> @@ -118,7 +118,7 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
>          env->priv_ver >= PRIV_VERSION_1_10_0 ?
>          MSTATUS_MIE : MSTATUS_UIE << prev_priv,
>          get_field(mstatus, MSTATUS_MPIE));
> -    mstatus = set_field(mstatus, MSTATUS_MPIE, 0);
> +    mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
>      mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
>      riscv_cpu_set_mode(env, prev_priv);
>      env->mstatus = mstatus;
> --
> 2.17.1
>
>

Re: [PATCH] riscv: Fix bug in setting xPIE of CSR for MRET and SRET instructions
Posted by Ian Jiang 4 years, 2 months ago
Just find that there is a previous patch at
https://github.com/palmer-dabbelt/qemu/commit/a37f21c27d3e2342c2080aafd4cfe7e949612428
--
Ian Jiang

Alistair Francis <alistair23@gmail.com> 于2020年1月21日周二 下午6:48写道:
>
> On Tue, Jan 21, 2020 at 8:08 PM Ian Jiang <ianjiang.ict@gmail.com> wrote:
> >
> > According to the RISC-V specification, when executing an MRET or SRET
> > instruction, xPIE in mstatus or sstatus should be set to 1. The orginal
> > QEMU does not give the right operations.
> > This patch fix the problem.
> >
> > Signed-off-by: Ian Jiang <ianjiang.ict@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
> > ---
> >  target/riscv/op_helper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index 331cc36232..e87c9115bc 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -93,7 +93,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
> >          env->priv_ver >= PRIV_VERSION_1_10_0 ?
> >          MSTATUS_SIE : MSTATUS_UIE << prev_priv,
> >          get_field(mstatus, MSTATUS_SPIE));
> > -    mstatus = set_field(mstatus, MSTATUS_SPIE, 0);
> > +    mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> >      mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> >      riscv_cpu_set_mode(env, prev_priv);
> >      env->mstatus = mstatus;
> > @@ -118,7 +118,7 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
> >          env->priv_ver >= PRIV_VERSION_1_10_0 ?
> >          MSTATUS_MIE : MSTATUS_UIE << prev_priv,
> >          get_field(mstatus, MSTATUS_MPIE));
> > -    mstatus = set_field(mstatus, MSTATUS_MPIE, 0);
> > +    mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
> >      mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
> >      riscv_cpu_set_mode(env, prev_priv);
> >      env->mstatus = mstatus;
> > --
> > 2.17.1
> >
> >