Re: [PATCH 01/12] target/s390x: Handle branching to odd addresses

[Ilya Leoshkevich]

On Fri, 2023-03-10 at 11:24 -0800, Richard Henderson wrote:
> On 3/10/23 09:42, Ilya Leoshkevich wrote:
> > @@ -381,6 +382,14 @@ static inline int cpu_mmu_index(CPUS390XState
> > *env, bool ifetch)
> >   static inline void cpu_get_tb_cpu_state(CPUS390XState* env,
> > target_ulong *pc,
> >                                           target_ulong *cs_base,
> > uint32_t *flags)
> >   {
> > +    if (env->psw.addr & 1) {
> > +        /*
> > +         * Instructions must be at even addresses.
> > +         * This needs to be checked before address translation.
> > +         */
> > +        env->int_pgm_ilen = 2; /* see s390_cpu_tlb_fill() */
> > +        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, 0);
> > +    }
> 
> This is incorrect placement.  You can't raise an exception from all
> of the places from 
> which this is called.
> 
> You need to do this at the start of s390x_tr_translate_insn.
> Compare aarch64_tr_translate_insn and the test for (pc & 3).
> 
> 
> r~

The problem is that it's too late - for non-mapped memory we would get
a translation exception instead of a specification exception.

I see the following call sites:

- HELPER(lookup_tb_ptr) - for helpers the exceptions should work;
- cpu_exec_loop(), cpu_exec_step_atomic - these are wrapped in setjmp,
  so it should be ok too?
- tb_check_watchpoint() - is this the problematic one?

Am I missing something?