| Message ID | 20190926162615.31168-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390: Use tcg unwinding for ilen | expand |
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~
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;