[PATCH 1/2] target/riscv: Simplify helper_sret() a little bit

2022-12-07 Thread Bin Meng 
There are 2 paths in helper_sret() and the same mstatus update codes
are replicated. Extract the common parts to simplify it a little bit.

Signed-off-by: Bin Meng 
---

 target/riscv/op_helper.c | 20 ++--
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index d7af7f056b..a047d38152 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -149,21 +149,21 @@ target_ulong helper_sret(CPURISCVState *env)
 }
 
 mstatus = env->mstatus;
+prev_priv = get_field(mstatus, MSTATUS_SPP);
+mstatus = set_field(mstatus, MSTATUS_SIE,
+get_field(mstatus, MSTATUS_SPIE));
+mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
+mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
+env->mstatus = mstatus;
 
 if (riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
 /* We support Hypervisor extensions and virtulisation is disabled */
 target_ulong hstatus = env->hstatus;
 
-prev_priv = get_field(mstatus, MSTATUS_SPP);
 prev_virt = get_field(hstatus, HSTATUS_SPV);
 
 hstatus = set_field(hstatus, HSTATUS_SPV, 0);
-mstatus = set_field(mstatus, MSTATUS_SPP, 0);
-mstatus = set_field(mstatus, SSTATUS_SIE,
-get_field(mstatus, SSTATUS_SPIE));
-mstatus = set_field(mstatus, SSTATUS_SPIE, 1);
 
-env->mstatus = mstatus;
 env->hstatus = hstatus;
 
 if (prev_virt) {
@@ -171,14 +171,6 @@ target_ulong helper_sret(CPURISCVState *env)
 }
 
 riscv_cpu_set_virt_enabled(env, prev_virt);
-} else {
-prev_priv = get_field(mstatus, MSTATUS_SPP);
-
-mstatus = set_field(mstatus, MSTATUS_SIE,
-get_field(mstatus, MSTATUS_SPIE));
-mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
-mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
-env->mstatus = mstatus;
 }
 
 riscv_cpu_set_mode(env, prev_priv);
-- 
2.34.1

Re: [PATCH 1/2] target/riscv: Simplify helper_sret() a little bit

2022-12-07 Thread Alistair Francis 
On Wed, Dec 7, 2022 at 7:05 PM Bin Meng  wrote:
>
> There are 2 paths in helper_sret() and the same mstatus update codes
> are replicated. Extract the common parts to simplify it a little bit.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  target/riscv/op_helper.c | 20 ++--
>  1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d7af7f056b..a047d38152 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -149,21 +149,21 @@ target_ulong helper_sret(CPURISCVState *env)
>  }
>
>  mstatus = env->mstatus;
> +prev_priv = get_field(mstatus, MSTATUS_SPP);
> +mstatus = set_field(mstatus, MSTATUS_SIE,
> +get_field(mstatus, MSTATUS_SPIE));
> +mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> +mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> +env->mstatus = mstatus;
>
>  if (riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
>  /* We support Hypervisor extensions and virtulisation is disabled */
>  target_ulong hstatus = env->hstatus;
>
> -prev_priv = get_field(mstatus, MSTATUS_SPP);
>  prev_virt = get_field(hstatus, HSTATUS_SPV);
>
>  hstatus = set_field(hstatus, HSTATUS_SPV, 0);
> -mstatus = set_field(mstatus, MSTATUS_SPP, 0);
> -mstatus = set_field(mstatus, SSTATUS_SIE,
> -get_field(mstatus, SSTATUS_SPIE));
> -mstatus = set_field(mstatus, SSTATUS_SPIE, 1);
>
> -env->mstatus = mstatus;
>  env->hstatus = hstatus;
>
>  if (prev_virt) {
> @@ -171,14 +171,6 @@ target_ulong helper_sret(CPURISCVState *env)
>  }
>
>  riscv_cpu_set_virt_enabled(env, prev_virt);
> -} else {
> -prev_priv = get_field(mstatus, MSTATUS_SPP);
> -
> -mstatus = set_field(mstatus, MSTATUS_SIE,
> -get_field(mstatus, MSTATUS_SPIE));
> -mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> -mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> -env->mstatus = mstatus;
>  }
>
>  riscv_cpu_set_mode(env, prev_priv);
> --
> 2.34.1
>
>

Re: [PATCH 1/2] target/riscv: Simplify helper_sret() a little bit

2022-12-08 Thread Wilfred Mallawa 
On Wed, 2022-12-07 at 17:00 +0800, Bin Meng wrote:
> There are 2 paths in helper_sret() and the same mstatus update codes
> are replicated. Extract the common parts to simplify it a little bit.
> 
> Signed-off-by: Bin Meng 
Reviewed-by: Wilfred Mallawa 

Wilfred
> ---
> 
>  target/riscv/op_helper.c | 20 ++--
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d7af7f056b..a047d38152 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -149,21 +149,21 @@ target_ulong helper_sret(CPURISCVState *env)
>  }
>  
>  mstatus = env->mstatus;
> +    prev_priv = get_field(mstatus, MSTATUS_SPP);
> +    mstatus = set_field(mstatus, MSTATUS_SIE,
> +    get_field(mstatus, MSTATUS_SPIE));
> +    mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> +    mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> +    env->mstatus = mstatus;
>  
>  if (riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
>  /* We support Hypervisor extensions and virtulisation is
> disabled */
>  target_ulong hstatus = env->hstatus;
>  
> -    prev_priv = get_field(mstatus, MSTATUS_SPP);
>  prev_virt = get_field(hstatus, HSTATUS_SPV);
>  
>  hstatus = set_field(hstatus, HSTATUS_SPV, 0);
> -    mstatus = set_field(mstatus, MSTATUS_SPP, 0);
> -    mstatus = set_field(mstatus, SSTATUS_SIE,
> -    get_field(mstatus, SSTATUS_SPIE));
> -    mstatus = set_field(mstatus, SSTATUS_SPIE, 1);
>  
> -    env->mstatus = mstatus;
>  env->hstatus = hstatus;
>  
>  if (prev_virt) {
> @@ -171,14 +171,6 @@ target_ulong helper_sret(CPURISCVState *env)
>  }
>  
>  riscv_cpu_set_virt_enabled(env, prev_virt);
> -    } else {
> -    prev_priv = get_field(mstatus, MSTATUS_SPP);
> -
> -    mstatus = set_field(mstatus, MSTATUS_SIE,
> -    get_field(mstatus, MSTATUS_SPIE));
> -    mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> -    mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> -    env->mstatus = mstatus;
>  }
>  
>  riscv_cpu_set_mode(env, prev_priv);