diff mbox series

[2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask

Message ID 20210615030744.1252385-3-richard.henderson@linaro.org
State Superseded
Headers show
Series linux-user/s390x: Fix psw.mask handling in signals | expand

Commit Message

Richard Henderson June 15, 2021, 3:07 a.m. UTC 
We want to use this function for debugging, and debug should
not modify cpu state (even non-architectural cpu state) lest
we introduce heisenbugs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/s390x/helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.25.1

Comments

David Hildenbrand June 15, 2021, 7:49 a.m. UTC | #1 
On 15.06.21 05:07, Richard Henderson wrote:
> We want to use this function for debugging, and debug should

> not modify cpu state (even non-architectural cpu state) lest

> we introduce heisenbugs.

> 

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>   target/s390x/helper.c | 8 ++++----

>   1 file changed, 4 insertions(+), 4 deletions(-)

> 

> diff --git a/target/s390x/helper.c b/target/s390x/helper.c

> index d311903b94..559fc3573f 100644

> --- a/target/s390x/helper.c

> +++ b/target/s390x/helper.c

> @@ -321,12 +321,12 @@ uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)

>       uint64_t r = env->psw.mask;

>   

>       if (tcg_enabled()) {

> -        env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,

> -                             env->cc_vr);

> +        uint64_t cc = calc_cc(env, env->cc_op, env->cc_src,

> +                              env->cc_dst, env->cc_vr);

>   

> +        assert(cc <= 3);

>           r &= ~PSW_MASK_CC;

> -        assert(!(env->cc_op & ~3));

> -        r |= (uint64_t)env->cc_op << 44;

> +        r |= cc << 44;

>       }

>   

>       return r;

> 


I wonder if this can actually suboptimal performance-wise, because we 
might be recomputing the same thing. However, I guess it's rather a 
corner case

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


-- 
Thanks,

David / dhildenb
diff mbox series

Patch

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index d311903b94..559fc3573f 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -321,12 +321,12 @@  uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
     uint64_t r = env->psw.mask;
 
     if (tcg_enabled()) {
-        env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
-                             env->cc_vr);
+        uint64_t cc = calc_cc(env, env->cc_op, env->cc_src,
+                              env->cc_dst, env->cc_vr);
 
+        assert(cc <= 3);
         r &= ~PSW_MASK_CC;
-        assert(!(env->cc_op & ~3));
-        r |= (uint64_t)env->cc_op << 44;
+        r |= cc << 44;
     }
 
     return r;