* [PATCH] Remove unassigned_access CPU hook
@ 2019-09-20 12:50 Peter Maydell
2019-09-20 13:36 ` Philippe Mathieu-Daudé
2019-09-20 18:45 ` Alistair Francis
0 siblings, 2 replies; 4+ messages in thread
From: Peter Maydell @ 2019-09-20 12:50 UTC (permalink / raw)
To: qemu-devel
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(-)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove unassigned_access CPU hook
2019-09-20 12:50 [PATCH] Remove unassigned_access CPU hook Peter Maydell
@ 2019-09-20 13:36 ` Philippe Mathieu-Daudé
2019-09-20 13:44 ` Peter Maydell
2019-09-20 18:45 ` Alistair Francis
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-20 13:36 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
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,
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove unassigned_access CPU hook
2019-09-20 13:36 ` Philippe Mathieu-Daudé
@ 2019-09-20 13:44 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2019-09-20 13:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: QEMU Developers
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove unassigned_access CPU hook
2019-09-20 12:50 [PATCH] Remove unassigned_access CPU hook Peter Maydell
2019-09-20 13:36 ` Philippe Mathieu-Daudé
@ 2019-09-20 18:45 ` Alistair Francis
1 sibling, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2019-09-20 18:45 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel@nongnu.org Developers
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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-20 18:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20 12:50 [PATCH] Remove unassigned_access CPU hook Peter Maydell
2019-09-20 13:36 ` Philippe Mathieu-Daudé
2019-09-20 13:44 ` Peter Maydell
2019-09-20 18:45 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).