Re: [PATCH v3 18/18] target/s390x: Remove ilen argument from trigger_pgm_exception

[David Hildenbrand]

On 26.09.19 18:26, Richard Henderson wrote:
> All but one caller passes ILEN_UNWIND, which is not stored.
> For the one use case in s390_cpu_tlb_fill, set int_pgm_ilen
> directly, simply to avoid the assert within do_program_interrupt.
> 
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/s390x/internal.h    | 2 +-
>  target/s390x/excp_helper.c | 7 ++++---
>  target/s390x/interrupt.c   | 7 ++-----
>  target/s390x/mmu_helper.c  | 2 +-
>  4 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index c993c3ef40..d37816104d 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -317,7 +317,7 @@ void cpu_unmap_lowcore(LowCore *lowcore);
>  
>  
>  /* interrupt.c */
> -void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen);
> +void trigger_pgm_exception(CPUS390XState *env, uint32_t code);
>  void cpu_inject_clock_comparator(S390CPU *cpu);
>  void cpu_inject_cpu_timer(S390CPU *cpu);
>  void cpu_inject_emergency_signal(S390CPU *cpu, uint16_t src_cpu_addr);
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index c252e9a7d8..e70c20d363 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -42,7 +42,7 @@ void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState 
> *env,
>      cpu_restore_state(cs, ra, true);
>      qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
>                    env->psw.addr);
> -    trigger_pgm_exception(env, code, ILEN_UNWIND);
> +    trigger_pgm_exception(env, code);
>      cpu_loop_exit(cs);
>  }
>  
> @@ -96,7 +96,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> size,
>  {
>      S390CPU *cpu = S390_CPU(cs);
>  
> -    trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_UNWIND);
> +    trigger_pgm_exception(&cpu->env, PGM_ADDRESSING);
>      /* On real machines this value is dropped into LowMem.  Since this
>         is userland, simply put this someplace that cpu_loop can find it.  */
>      cpu->env.__excp_addr = address;
> @@ -186,7 +186,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> size,
>       * and so unwinding will not occur.  However, ILEN is also undefined
>       * for that case -- we choose to set ILEN = 2.
>       */
> -    trigger_pgm_exception(env, excp, 2);
> +    env->int_pgm_ilen = 2;
> +    trigger_pgm_exception(env, excp);
>      cpu_loop_exit_restore(cs, retaddr);
>  }
>  
> diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
> index 2b71e03914..4cdbbc8849 100644
> --- a/target/s390x/interrupt.c
> +++ b/target/s390x/interrupt.c
> @@ -22,16 +22,13 @@
>  #endif
>  
>  /* Ensure to exit the TB after this call! */
> -void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
> +void trigger_pgm_exception(CPUS390XState *env, uint32_t code)
>  {
>      CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = EXCP_PGM;
>      env->int_pgm_code = code;
> -    /* If ILEN_UNWIND, int_pgm_ilen already has the correct value.  */
> -    if (ilen != ILEN_UNWIND) {
> -        env->int_pgm_ilen = ilen;
> -    }
> +    /* env->int_pgm_ilen is already set, or will be set during unwinding */
>  }
>  
>  void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra)
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index e6c3139c57..ba02d33e86 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -44,7 +44,7 @@ static void trigger_access_exception(CPUS390XState *env, 
> uint32_t type,
>          if (type != PGM_ADDRESSING) {
>              stq_phys(cs->as, env->psa + offsetof(LowCore, trans_exc_code), 
> tec);
>          }
> -        trigger_pgm_exception(env, type, ILEN_UNWIND);
> +        trigger_pgm_exception(env, type);
>      }
>  }
>  
> 

Reviewed-by: David Hildenbrand <address@hidden>

-- 

Thanks,

David / dhildenb