Message ID | 20190920125008.13604-1-peter.maydell@linaro.org |
---|---|
State | New |
Headers | show |
Series | Remove unassigned_access CPU hook | expand |
Hi Peter, On 9/20/19 2:50 PM, Peter Maydell 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 <peter.maydell@linaro.org> > --- > Based-on: <cover.1568762497.git.alistair.francis@wdc.com> > ("[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. Hopefully this hook is neither implemented by the RX/AVR targets posted on the list recently. > 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); > - } > } Having unassigned_mem_read/unassigned_mem_write with CPUReadMemoryFunc/CPUWriteMemoryFunc prototype only used for logging is rather confusing. We can kill them and use trace events instead in memory_region_dispatch_read/write. I'll send a follow-up cleanup patch. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > static bool unassigned_mem_accepts(void *opaque, hwaddr addr, >
On Fri, 20 Sep 2019 at 14:36, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Hi Peter, > > On 9/20/19 2:50 PM, Peter Maydell 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 <peter.maydell@linaro.org> > > --- > > Based-on: <cover.1568762497.git.alistair.francis@wdc.com> > > ("[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. > > Hopefully this hook is neither implemented by the RX/AVR targets posted > on the list recently. Good point -- luckily a quick email archive search says they don't try to implement the old hook. > > @@ -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); > > - } > > } > > Having unassigned_mem_read/unassigned_mem_write with > CPUReadMemoryFunc/CPUWriteMemoryFunc prototype only used for logging is > rather confusing. We can kill them and use trace events instead in > memory_region_dispatch_read/write. I'll send a follow-up cleanup patch. You still need some function to do the "return 0" on read, though, don't you? (Having unassigned_mem_accepts returning false also leaves me a bit confused about when these functions would actually get called, now I look at the code again...) thanks -- PMM
On Fri, Sep 20, 2019 at 5:52 AM Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > Based-on: <cover.1568762497.git.alistair.francis@wdc.com> > ("[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 > >
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,
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 <peter.maydell@linaro.org> --- Based-on: <cover.1568762497.git.alistair.francis@wdc.com> ("[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(-)