Re: [PATCH 1/2] target/s390x: Fix EXECUTE of relative long instructions

[David Hildenbrand <david@redhat.com>]

On 14.03.23 00:38, Ilya Leoshkevich wrote:
> The code uses the wrong base for relative addressing: it should use the
> target instruction address and not the EXECUTE's address.
> 
> Fix by storing the target instruction address in the new CPUS390XState
> member and loading it from the code generated by in2_ri2().
> 
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/cpu.h            |  1 +
>   target/s390x/tcg/mem_helper.c |  1 +
>   target/s390x/tcg/translate.c  | 10 +++++++++-
>   3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 7d6d01325b2..8aaf8dd5a3b 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -87,6 +87,7 @@ struct CPUArchState {
>       uint64_t cc_vr;
>   
>       uint64_t ex_value;
> +    uint64_t ex_target;
>   
>       uint64_t __excp_addr;
>       uint64_t psa;
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index 6835c26dda4..00afae2b640 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -2530,6 +2530,7 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
>          that ex_value is non-zero, which flags that we are in a state
>          that requires such execution.  */
>       env->ex_value = insn | ilen;
> +    env->ex_target = addr;
>   }
>   
>   uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 811049ea281..fefff95b91c 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -5888,7 +5888,15 @@ static void in2_a2(DisasContext *s, DisasOps *o)
>   
>   static void in2_ri2(DisasContext *s, DisasOps *o)
>   {
> -    o->in2 = tcg_const_i64(s->base.pc_next + (int64_t)get_field(s, i2) * 2);
> +    int64_t delta = (int64_t)get_field(s, i2) * 2;
> +
> +    if (unlikely(s->ex_value)) {
> +        o->in2 = tcg_temp_new_i64();
> +        tcg_gen_ld_i64(o->in2, cpu_env, offsetof(CPUS390XState, ex_target));
> +        tcg_gen_addi_i64(o->in2, o->in2, delta);
> +    } else {
> +        o->in2 = tcg_const_i64(s->base.pc_next + delta);
> +    }
>   }
>   #define SPEC_in2_ri2 0
>   

LGTM, hopefully Richard can have another look. Thanks!

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

-- 
Thanks,

David / dhildenb