Re: [PATCH] Remove unassigned_access CPU hook

[Alistair Francis]

On Fri, Sep 20, 2019 at 5:52 AM Peter Maydell <address@hidden> wrote:
>
> All targets have now migrated away from the old unassigned_access
> hook to the new do_transaction_failed hook. This means we can remove
> the core-code infrastructure for that hook and the code that calls it.
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
> Based-on: <address@hidden>
> ("[PATCH v1 0/2] RISC-V: Convert to do_transaction_failed hook")
>  -- the last of the conversions isn't in master yet, but I figured
> I might as well put the cleanup patch out for review.
>
>  include/hw/core/cpu.h | 24 ------------------------
>  accel/tcg/cputlb.c    |  1 -
>  memory.c              |  7 -------
>  3 files changed, 32 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index c7cda65c66d..a5a13e26a20 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -71,10 +71,6 @@ typedef enum MMUAccessType {
>
>  typedef struct CPUWatchpoint CPUWatchpoint;
>
> -typedef void (*CPUUnassignedAccess)(CPUState *cpu, hwaddr addr,
> -                                    bool is_write, bool is_exec, int opaque,
> -                                    unsigned size);
> -
>  struct TranslationBlock;
>
>  /**
> @@ -86,8 +82,6 @@ struct TranslationBlock;
>   * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
>   * @has_work: Callback for checking if there is work to do.
>   * @do_interrupt: Callback for interrupt handling.
> - * @do_unassigned_access: Callback for unassigned access handling.
> - * (this is deprecated: new targets should use do_transaction_failed instead)
>   * @do_unaligned_access: Callback for unaligned access handling, if
>   * the target defines #TARGET_ALIGNED_ONLY.
>   * @do_transaction_failed: Callback for handling failed memory transactions
> @@ -174,7 +168,6 @@ typedef struct CPUClass {
>      int reset_dump_flags;
>      bool (*has_work)(CPUState *cpu);
>      void (*do_interrupt)(CPUState *cpu);
> -    CPUUnassignedAccess do_unassigned_access;
>      void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
>                                  MMUAccessType access_type,
>                                  int mmu_idx, uintptr_t retaddr);
> @@ -414,12 +407,6 @@ struct CPUState {
>       */
>      uintptr_t mem_io_pc;
>      vaddr mem_io_vaddr;
> -    /*
> -     * This is only needed for the legacy cpu_unassigned_access() hook;
> -     * when all targets using it have been converted to use
> -     * cpu_transaction_failed() instead it can be removed.
> -     */
> -    MMUAccessType mem_io_access_type;
>
>      int kvm_fd;
>      struct KVMState *kvm_state;
> @@ -879,17 +866,6 @@ void cpu_interrupt(CPUState *cpu, int mask);
>  #ifdef NEED_CPU_H
>
>  #ifdef CONFIG_SOFTMMU
> -static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> -                                         bool is_write, bool is_exec,
> -                                         int opaque, unsigned size)
> -{
> -    CPUClass *cc = CPU_GET_CLASS(cpu);
> -
> -    if (cc->do_unassigned_access) {
> -        cc->do_unassigned_access(cpu, addr, is_write, is_exec, opaque, size);
> -    }
> -}
> -
>  static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr,
>                                          MMUAccessType access_type,
>                                          int mmu_idx, uintptr_t retaddr)
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index abae79650c0..9c21b320eb4 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -914,7 +914,6 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>      }
>
>      cpu->mem_io_vaddr = addr;
> -    cpu->mem_io_access_type = access_type;
>
>      if (mr->global_locking && !qemu_mutex_iothread_locked()) {
>          qemu_mutex_lock_iothread();
> diff --git a/memory.c b/memory.c
> index b9dd6b94cac..93a05395cff 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1278,10 +1278,6 @@ static uint64_t unassigned_mem_read(void *opaque, 
> hwaddr addr,
>  #ifdef DEBUG_UNASSIGNED
>      printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>  #endif
> -    if (current_cpu != NULL) {
> -        bool is_exec = current_cpu->mem_io_access_type == MMU_INST_FETCH;
> -        cpu_unassigned_access(current_cpu, addr, false, is_exec, 0, size);
> -    }
>      return 0;
>  }
>
> @@ -1291,9 +1287,6 @@ static void unassigned_mem_write(void *opaque, hwaddr 
> addr,
>  #ifdef DEBUG_UNASSIGNED
>      printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, 
> val);
>  #endif
> -    if (current_cpu != NULL) {
> -        cpu_unassigned_access(current_cpu, addr, true, false, 0, size);
> -    }
>  }
>
>  static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
> --
> 2.20.1
>
>