Re: [PATCH v5 52/67] target/s390x: Implement s390x_cpu_record_sigbus

[Philippe Mathieu-Daudé]

On 10/15/21 06:10, Richard Henderson wrote:
> For s390x, the only unaligned accesses that are signaled are atomic,
> and we don't actually want to raise SIGBUS for those, but instead
> raise a SPECIFICATION error, which the kernel will report as SIGILL.
> 
> Split out a do_unaligned_access function to share between the user-only
> s390x_cpu_record_sigbus and the sysemu s390x_do_unaligned_access.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/s390x/s390x-internal.h  |  8 +++++---
>  target/s390x/cpu.c             |  1 +
>  target/s390x/tcg/excp_helper.c | 27 ++++++++++++++++++++-------
>  3 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index 163aa4f94a..1a178aed41 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -270,18 +270,20 @@ ObjectClass *s390_cpu_class_by_name(const char *name);
>  void s390x_cpu_debug_excp_handler(CPUState *cs);
>  void s390_cpu_do_interrupt(CPUState *cpu);
>  bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);
> -void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> -                                   MMUAccessType access_type, int mmu_idx,
> -                                   uintptr_t retaddr) QEMU_NORETURN;
>  
>  #ifdef CONFIG_USER_ONLY
>  void s390_cpu_record_sigsegv(CPUState *cs, vaddr address,
>                               MMUAccessType access_type,
>                               bool maperr, uintptr_t retaddr);
> +void s390_cpu_record_sigbus(CPUState *cs, vaddr address,
> +                            MMUAccessType access_type, uintptr_t retaddr);
>  #else
>  bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                         MMUAccessType access_type, int mmu_idx,
>                         bool probe, uintptr_t retaddr);
> +void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> +                                   MMUAccessType access_type, int mmu_idx,
> +                                   uintptr_t retaddr) QEMU_NORETURN;
>  #endif
>  
>  
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 593dda75c4..ccdbaf84d5 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -269,6 +269,7 @@ static const struct TCGCPUOps s390_tcg_ops = {
>  
>  #ifdef CONFIG_USER_ONLY
>      .record_sigsegv = s390_cpu_record_sigsegv,
> +    .record_sigbus = s390_cpu_record_sigbus,
>  #else
>      .tlb_fill = s390_cpu_tlb_fill,
>      .cpu_exec_interrupt = s390_cpu_exec_interrupt,
> diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
> index b923d080fc..4e7648f301 100644
> --- a/target/s390x/tcg/excp_helper.c
> +++ b/target/s390x/tcg/excp_helper.c
> @@ -82,6 +82,19 @@ void HELPER(data_exception)(CPUS390XState *env, uint32_t 
> dxc)
>      tcg_s390_data_exception(env, dxc, GETPC());
>  }
>  
> +/*
> + * Unaligned accesses are only diagnosed with MO_ALIGN.  At the moment,
> + * this is only for the atomic operations, for which we want to raise a
> + * specification exception.
> + */
> +static void QEMU_NORETURN do_unaligned_access(CPUState *cs, uintptr_t 
> retaddr)
> +{
> +    S390CPU *cpu = S390_CPU(cs);
> +    CPUS390XState *env = &cpu->env;
> +
> +    tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr);
> +}
> +
>  #if defined(CONFIG_USER_ONLY)
>  
>  void s390_cpu_do_interrupt(CPUState *cs)
> @@ -106,6 +119,12 @@ void s390_cpu_record_sigsegv(CPUState *cs, vaddr address,
>      cpu_loop_exit_restore(cs, retaddr);
>  }
>  
> +void s390_cpu_record_sigbus(CPUState *cs, vaddr address,
> +                            MMUAccessType access_type, uintptr_t retaddr)
> +{
> +    do_unaligned_access(cs, retaddr);
> +}
> +
>  #else /* !CONFIG_USER_ONLY */
>  
>  static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)
> @@ -593,17 +612,11 @@ void s390x_cpu_debug_excp_handler(CPUState *cs)
>      }
>  }
>  
> -/* Unaligned accesses are only diagnosed with MO_ALIGN.  At the moment,
> -   this is only for the atomic operations, for which we want to raise a
> -   specification exception.  */
>  void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>                                     MMUAccessType access_type,
>                                     int mmu_idx, uintptr_t retaddr)
>  {
> -    S390CPU *cpu = S390_CPU(cs);
> -    CPUS390XState *env = &cpu->env;
> -
> -    tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr);
> +    do_unaligned_access(cs, retaddr);
>  }
>  
>  static void QEMU_NORETURN monitor_event(CPUS390XState *env,
>