If, somehow, the psw_addr is out of range, truncate early
rather than after we get into gen_intermediate_code.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/cpu.h       | 26 +++++++++++++++++++-------
 target/s390x/translate.c |  6 ------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index e74a809257..ce20dafd23 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -357,18 +357,30 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
 #endif
 }
 
-static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
-                                        target_ulong *cs_base, uint32_t *flags)
+static inline void cpu_get_tb_cpu_state(CPUS390XState* env,
+                                        target_ulong *p_pc,
+                                        target_ulong *cs_base,
+                                        uint32_t *p_flags)
 {
-    *pc = env->psw.addr;
-    *cs_base = env->ex_value;
-    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
+    uint32_t flags;
+    uint64_t pc;
+
+    flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
     if (env->cregs[0] & CR0_AFP) {
-        *flags |= FLAG_MASK_AFP;
+        flags |= FLAG_MASK_AFP;
     }
     if (env->cregs[0] & CR0_VECTOR) {
-        *flags |= FLAG_MASK_VECTOR;
+        flags |= FLAG_MASK_VECTOR;
     }
+
+    pc = env->psw.addr;
+    if (!(flags & FLAG_MASK_64)) {
+        pc &= 0x7fffffff;
+    }
+
+    *p_pc = pc;
+    *cs_base = env->ex_value;
+    *p_flags = flags;
 }
 
 /* PER bits from control register 9 */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a3e43ff9ec..e1c54ab03b 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6446,12 +6446,6 @@ static void s390x_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
-    /* 31-bit mode */
-    if (!(dc->base.tb->flags & FLAG_MASK_64)) {
-        dc->base.pc_first &= 0x7fffffff;
-        dc->base.pc_next = dc->base.pc_first;
-    }
-
     dc->cc_op = CC_OP_DYNAMIC;
     dc->ex_value = dc->base.tb->cs_base;
     dc->do_debug = dc->base.singlestep_enabled;
-- 
2.17.1

On 26.09.19 18:25, Richard Henderson wrote:
> If, somehow, the psw_addr is out of range, truncate early
> rather than after we get into gen_intermediate_code.
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/cpu.h       | 26 +++++++++++++++++++-------
>  target/s390x/translate.c |  6 ------
>  2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index e74a809257..ce20dafd23 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -357,18 +357,30 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
>  #endif
>  }
>  
> -static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
> -                                        target_ulong *cs_base, uint32_t *flags)
> +static inline void cpu_get_tb_cpu_state(CPUS390XState* env,
> +                                        target_ulong *p_pc,
> +                                        target_ulong *cs_base,
> +                                        uint32_t *p_flags)
>  {
> -    *pc = env->psw.addr;
> -    *cs_base = env->ex_value;
> -    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
> +    uint32_t flags;
> +    uint64_t pc;
> +
> +    flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
>      if (env->cregs[0] & CR0_AFP) {
> -        *flags |= FLAG_MASK_AFP;
> +        flags |= FLAG_MASK_AFP;
>      }
>      if (env->cregs[0] & CR0_VECTOR) {
> -        *flags |= FLAG_MASK_VECTOR;
> +        flags |= FLAG_MASK_VECTOR;
>      }
> +
> +    pc = env->psw.addr;
> +    if (!(flags & FLAG_MASK_64)) {
> +        pc &= 0x7fffffff;
> +    }

If you're fancy, you could also add 24-bit addressing mode wrapping.

Maybe unlikely(!(flags & FLAG_MASK_64)), but not sure how big the gain
will actually be.

-- 

Thanks,

David / dhildenb

On 9/27/19 3:23 AM, David Hildenbrand wrote:
>> +    pc = env->psw.addr;
>> +    if (!(flags & FLAG_MASK_64)) {
>> +        pc &= 0x7fffffff;
>> +    }
> 
> If you're fancy, you could also add 24-bit addressing mode wrapping.
> 
> Maybe unlikely(!(flags & FLAG_MASK_64)), but not sure how big the gain
> will actually be.

So, it appears that this patch, and the existing code in
s390x_tr_init_disas_context, are wrong.

Page 4-7 of the v11 PoO says that we must generate a specification exception
and not wrap.  I believe that the only way to force this to happen is with an
explicit LOAD PSW EXTENDED instruction.  Branches and sequential instruction
execution both wrap the addresses before writing back to the PSW.

I will drop this patch for now and we'll look again at instruction address later.


r~