Re: [Qemu-devel] [PATCH v2 10/19] target/s390x: use program_interrupt() in per_check_exception()

[Cornelia Huck <cohuck@redhat.com>]

On Mon,  4 Sep 2017 17:43:07 +0200
David Hildenbrand <david@redhat.com> wrote:

> Clean it up by reusing program_interrupt(). Add a concern regarding
> ilen.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/misc_helper.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 57c02ddf1b..5096286157 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -446,14 +446,17 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
>  #ifndef CONFIG_USER_ONLY
>  void HELPER(per_check_exception)(CPUS390XState *env)
>  {
> -    CPUState *cs = CPU(s390_env_get_cpu(env));
> +    uint32_t ilen;
>  
>      if (env->per_perc_atmid) {
> -        env->int_pgm_code = PGM_PER;
> -        env->int_pgm_ilen = get_ilen(cpu_ldub_code(env, env->per_address));
> -
> -        cs->exception_index = EXCP_PGM;
> -        cpu_loop_exit(cs);
> +        /*
> +         * FIXME: ILEN_AUTO is most probably the right thing to use. ilen
> +         * always has to match the instruction referenced in the PSW. E.g.
> +         * if a PER interrupt is triggered via EXECUTE, we have to use ilen
> +         * of EXECUTE, while per_address contains the target of EXECUTE.
> +         */
> +        ilen = get_ilen(cpu_ldub_code(env, env->per_address));
> +        program_interrupt(env, PGM_PER, ilen);
>      }
>  }
>  

Wrapping my head around it: You preserve the current behavior, which
should probably be changed to use ILEN_AUTO to handle cases like
EXECUTE correctly?

Do you plan to do a follow up? (Should I spend some cycles on it?)