[v3,14/18] target/s390x: Tidy SRST
diff mbox

Message ID 20170620000405.3391-15-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson June 20, 2017, 12:04 a.m. UTC
Since we require all registers saved on input, read R0 from ENV instead
of passing it manually.  Recognize the specification exception when R0
contains incorrect data.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/helper.h     |  2 +-
 target/s390x/mem_helper.c | 11 ++++++++---
 target/s390x/translate.c  |  2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

Comments

David Hildenbrand June 20, 2017, 7:33 a.m. UTC | #1
On 20.06.2017 02:04, Richard Henderson wrote:
> Since we require all registers saved on input, read R0 from ENV instead
> of passing it manually.  Recognize the specification exception when R0
> contains incorrect data.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Reviewed-by: David Hildenbrand <david@redhat.com>

> ---
>  target/s390x/helper.h     |  2 +-
>  target/s390x/mem_helper.c | 11 ++++++++---
>  target/s390x/translate.c  |  2 +-
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index c014820..cd51b89 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -12,7 +12,7 @@ DEF_HELPER_FLAGS_3(divs32, TCG_CALL_NO_WG, s64, env, s64, s64)
>  DEF_HELPER_FLAGS_3(divu32, TCG_CALL_NO_WG, i64, env, i64, i64)
>  DEF_HELPER_FLAGS_3(divs64, TCG_CALL_NO_WG, s64, env, s64, s64)
>  DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
> -DEF_HELPER_4(srst, i64, env, i64, i64, i64)
> +DEF_HELPER_3(srst, i64, env, i64, i64)
>  DEF_HELPER_4(clst, i64, env, i64, i64, i64)
>  DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
>  DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index df082f5..990858e 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -538,12 +538,17 @@ static inline void set_length(CPUS390XState *env, int reg, uint64_t length)
>  }
>  
>  /* search string (c is byte to search, r2 is string, r1 end of string) */
> -uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
> -                      uint64_t str)
> +uint64_t HELPER(srst)(CPUS390XState *env, uint64_t end, uint64_t str)
>  {
>      uintptr_t ra = GETPC();
>      uint32_t len;
> -    uint8_t v, c = r0;
> +    uint8_t v, c = env->regs[0];
> +
> +    /* Bits 32-55 must contain all 0.  */
> +    if (env->regs[0] & 0xffffff00u) {
> +        cpu_restore_state(ENV_GET_CPU(env), ra);
> +        program_interrupt(env, PGM_SPECIFICATION, 6);
> +    }
>  
>      str = wrap_address(env, str);
>      end = wrap_address(env, end);
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index f8989ec..4a860f1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -4256,7 +4256,7 @@ static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
>  
>  static ExitStatus op_srst(DisasContext *s, DisasOps *o)
>  {
> -    gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
> +    gen_helper_srst(o->in1, cpu_env, o->in1, o->in2);
>      set_cc_static(s);
>      return_low128(o->in2);
>      return NO_EXIT;
>
Aurelien Jarno June 23, 2017, 3:52 p.m. UTC | #2
On 2017-06-19 17:04, Richard Henderson wrote:
> Since we require all registers saved on input, read R0 from ENV instead
> of passing it manually.  Recognize the specification exception when R0
> contains incorrect data.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/helper.h     |  2 +-
>  target/s390x/mem_helper.c | 11 ++++++++---
>  target/s390x/translate.c  |  2 +-
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index c014820..cd51b89 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -12,7 +12,7 @@ DEF_HELPER_FLAGS_3(divs32, TCG_CALL_NO_WG, s64, env, s64, s64)
>  DEF_HELPER_FLAGS_3(divu32, TCG_CALL_NO_WG, i64, env, i64, i64)
>  DEF_HELPER_FLAGS_3(divs64, TCG_CALL_NO_WG, s64, env, s64, s64)
>  DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
> -DEF_HELPER_4(srst, i64, env, i64, i64, i64)
> +DEF_HELPER_3(srst, i64, env, i64, i64)
>  DEF_HELPER_4(clst, i64, env, i64, i64, i64)
>  DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
>  DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index df082f5..990858e 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -538,12 +538,17 @@ static inline void set_length(CPUS390XState *env, int reg, uint64_t length)
>  }
>  
>  /* search string (c is byte to search, r2 is string, r1 end of string) */
> -uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
> -                      uint64_t str)
> +uint64_t HELPER(srst)(CPUS390XState *env, uint64_t end, uint64_t str)
>  {
>      uintptr_t ra = GETPC();
>      uint32_t len;
> -    uint8_t v, c = r0;
> +    uint8_t v, c = env->regs[0];
> +
> +    /* Bits 32-55 must contain all 0.  */
> +    if (env->regs[0] & 0xffffff00u) {
> +        cpu_restore_state(ENV_GET_CPU(env), ra);
> +        program_interrupt(env, PGM_SPECIFICATION, 6);
> +    }
>  
>      str = wrap_address(env, str);
>      end = wrap_address(env, end);
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index f8989ec..4a860f1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -4256,7 +4256,7 @@ static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
>  
>  static ExitStatus op_srst(DisasContext *s, DisasOps *o)
>  {
> -    gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
> +    gen_helper_srst(o->in1, cpu_env, o->in1, o->in2);
>      set_cc_static(s);
>      return_low128(o->in2);
>      return NO_EXIT;

The cleanup is a good step, but I guess that should also be the moment
to improve the address masking/wrapping (see comment on next patch).

Anyway:

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

Patch
diff mbox

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index c014820..cd51b89 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -12,7 +12,7 @@  DEF_HELPER_FLAGS_3(divs32, TCG_CALL_NO_WG, s64, env, s64, s64)
 DEF_HELPER_FLAGS_3(divu32, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(divs64, TCG_CALL_NO_WG, s64, env, s64, s64)
 DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
-DEF_HELPER_4(srst, i64, env, i64, i64, i64)
+DEF_HELPER_3(srst, i64, env, i64, i64)
 DEF_HELPER_4(clst, i64, env, i64, i64, i64)
 DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index df082f5..990858e 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -538,12 +538,17 @@  static inline void set_length(CPUS390XState *env, int reg, uint64_t length)
 }
 
 /* search string (c is byte to search, r2 is string, r1 end of string) */
-uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
-                      uint64_t str)
+uint64_t HELPER(srst)(CPUS390XState *env, uint64_t end, uint64_t str)
 {
     uintptr_t ra = GETPC();
     uint32_t len;
-    uint8_t v, c = r0;
+    uint8_t v, c = env->regs[0];
+
+    /* Bits 32-55 must contain all 0.  */
+    if (env->regs[0] & 0xffffff00u) {
+        cpu_restore_state(ENV_GET_CPU(env), ra);
+        program_interrupt(env, PGM_SPECIFICATION, 6);
+    }
 
     str = wrap_address(env, str);
     end = wrap_address(env, end);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f8989ec..4a860f1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4256,7 +4256,7 @@  static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_srst(DisasContext *s, DisasOps *o)
 {
-    gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
+    gen_helper_srst(o->in1, cpu_env, o->in1, o->in2);
     set_cc_static(s);
     return_low128(o->in2);
     return NO_EXIT;