In preparation for the conversion to an RCU controlled list of breakpoints I've removed all $ARCH local references to breakpoint structures. They can be accessed with cpu_breakpoint_get_by_ref() which will eventually offer them for the lifetime of the rcu_read_lock().
Instead of using pointers as handles to architecture specific registers they now use plain integer references. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- exec.c | 116 +++++++++++++++++++++++++++++++---------------- gdbstub.c | 2 +- include/qom/cpu.h | 59 +++++++++++++++++++++--- linux-user/main.c | 2 +- target-arm/cpu.h | 2 - target-arm/helper.c | 12 ++--- target-arm/op_helper.c | 6 +-- target-i386/bpt_helper.c | 25 ++++------ target-i386/cpu.h | 3 -- target-lm32/cpu.h | 2 - target-lm32/helper.c | 10 +--- 11 files changed, 148 insertions(+), 91 deletions(-) diff --git a/exec.c b/exec.c index 3dc3332..e80c9fe 100644 --- a/exec.c +++ b/exec.c @@ -797,7 +797,7 @@ int cpu_watchpoint_insert_with_ref(CPUState *cpu, vaddr addr, vaddr len, } /* Find old watchpoint */ - if (ref != WP_NOREF) { + if (ref != BPWP_NOREF) { wp = cpu_watchpoint_get_by_ref(cpu, ref); } @@ -830,7 +830,7 @@ int cpu_watchpoint_insert_with_ref(CPUState *cpu, vaddr addr, vaddr len, int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, int flags) { - return cpu_watchpoint_insert_with_ref(cpu, addr, len, flags, WP_NOREF); + return cpu_watchpoint_insert_with_ref(cpu, addr, len, flags, BPWP_NOREF); } static void cpu_watchpoint_delete(CPUState *cpu, int index) @@ -921,10 +921,20 @@ static inline bool cpu_watchpoint_address_matches(CPUWatchpoint *wp, } #endif +/* Find watchpoint with external reference */ +CPUBreakpoint *cpu_breakpoint_get_by_ref(CPUState *cpu, int ref) +{ + CPUBreakpoint *bp = NULL; + int i = 0; + do { + bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i++); + } while (bp && bp->ref != ref); + + return bp; +} /* Add a breakpoint. */ -int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, - CPUBreakpoint **breakpoint) +int cpu_breakpoint_insert_with_ref(CPUState *cpu, vaddr pc, int flags, int ref) { CPUBreakpoint *bp; @@ -933,76 +943,102 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, cpu->breakpoints = g_array_new(false, false, sizeof(CPUBreakpoint *)); } - bp = g_malloc(sizeof(*bp)); - - bp->pc = pc; - bp->flags = flags; + /* Find old watchpoint */ + if (ref != BPWP_NOREF) { + bp = cpu_breakpoint_get_by_ref(cpu, ref); + } - /* keep all GDB-injected breakpoints in front */ - if (flags & BP_GDB) { - g_array_prepend_val(cpu->breakpoints, bp); + if (bp) { + bp->pc = pc; + bp->flags = flags; + bp->ref = ref; } else { - g_array_append_val(cpu->breakpoints, bp); + bp = g_malloc(sizeof(*bp)); + + bp->pc = pc; + bp->flags = flags; + bp->ref = ref; + + /* keep all GDB-injected breakpoints in front */ + if (flags & BP_GDB) { + g_array_prepend_val(cpu->breakpoints, bp); + } else { + g_array_append_val(cpu->breakpoints, bp); + } } breakpoint_invalidate(cpu, pc); - if (breakpoint) { - *breakpoint = bp; - } return 0; } +int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags) +{ + return cpu_breakpoint_insert_with_ref(cpu, pc, flags, BPWP_NOREF); +} + +static void cpu_breakpoint_delete(CPUState *cpu, int index) +{ + CPUBreakpoint *bp; + bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, index); + g_array_remove_index(cpu->breakpoints, index); + breakpoint_invalidate(cpu, bp->pc); + g_free(bp); +} + /* Remove a specific breakpoint. */ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags) { CPUBreakpoint *bp; - int i; - g_assert(cpu->breakpoints); - - for (i = 0; i < cpu->breakpoints->len; i++) { - bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i); - if (bp->pc == pc && bp->flags == flags) { - cpu_breakpoint_remove_by_ref(cpu, bp); - return 0; - } + if (unlikely(cpu->breakpoints) && unlikely(cpu->breakpoints->len)) { + int i = 0; + do { + bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i); + if (bp && bp->pc == pc && bp->flags == flags) { + cpu_breakpoint_delete(cpu, i); + } else { + i++; + } + } while (i < cpu->breakpoints->len); } + return -ENOENT; } /* Remove a specific breakpoint by reference. */ -void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint) +void cpu_breakpoint_remove_by_ref(CPUState *cpu, int ref) { CPUBreakpoint *bp; - int i; - for (i = 0; i < cpu->breakpoints->len; i++) { - bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i); - if (bp == breakpoint) { - g_array_remove_index_fast(cpu->breakpoints, i); - break; - } + if (unlikely(cpu->breakpoints) && unlikely(cpu->breakpoints->len)) { + int i = 0; + do { + bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i); + if (bp && bp->ref == ref) { + cpu_breakpoint_delete(cpu, i); + } else { + i++; + } + } while (i < cpu->breakpoints->len); } - - breakpoint_invalidate(cpu, breakpoint->pc); - - g_free(breakpoint); } /* Remove all matching breakpoints. */ void cpu_breakpoint_remove_all(CPUState *cpu, int mask) { CPUBreakpoint *bp; - int i; if (unlikely(cpu->breakpoints) && unlikely(cpu->breakpoints->len)) { - for (i = cpu->breakpoints->len; i == 0; i--) { + int i = 0; + do { bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i); if (bp->flags & mask) { - cpu_breakpoint_remove_by_ref(cpu, bp); + cpu_breakpoint_delete(cpu, i); + } else { + i++; } - } + } while (i < cpu->breakpoints->len); } } diff --git a/gdbstub.c b/gdbstub.c index 548144e..c73ea42 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -676,7 +676,7 @@ static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type) case GDB_BREAKPOINT_SW: case GDB_BREAKPOINT_HW: CPU_FOREACH(cpu) { - err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL); + err = cpu_breakpoint_insert(cpu, addr, BP_GDB); if (err) { break; } diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 81edfdb..e582da0 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -201,20 +201,21 @@ typedef struct icount_decr_u16 { } icount_decr_u16; #endif +#define BPWP_NOREF -1 + typedef struct CPUBreakpoint { vaddr pc; int flags; /* BP_* */ + int ref; /* reference or WPBP_NOREF */ } CPUBreakpoint; -#define WP_NOREF -1 - struct CPUWatchpoint { vaddr vaddr; vaddr len; vaddr hitaddr; MemTxAttrs hitattrs; int flags; /* BP_* */ - int ref; /* reference or WP_NOREF */ + int ref; /* reference or WPBP_NOREF */ }; struct KVMState; @@ -818,10 +819,56 @@ void cpu_single_step(CPUState *cpu, int enabled); #define BP_WATCHPOINT_HIT_WRITE 0x80 #define BP_WATCHPOINT_HIT (BP_WATCHPOINT_HIT_READ | BP_WATCHPOINT_HIT_WRITE) -int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, - CPUBreakpoint **breakpoint); +/* + * cpu_breakpoint_insert: + * + * @cpu: CPU to monitor + * @pc: address of breakpoint + * @flags: accesss/type flags + * + * Breakpoints added this way can only be removed by resetting all + * breakpoints. + */ +int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags); + +/* + * cpu_breakpoint_insert_with_ref: + * + * @cpu: CPU to monitor + * @pc: address of breakpoint + * @flags: accesss/type flags + * + * Inserting a breakpoint with a matching reference will replace any + * current breakpoint with the same reference. + */ +int cpu_breakpoint_insert_with_ref(CPUState *cpu, vaddr pc, int flags, int ref); + +/** + * cpu_breakpoint_get_by_ref: + * + * @cpu: CPU to monitor + * @ref: unique reference + * + * @return: a pointer to working copy of the breakpoint. + * + * Return a working copy of the current referenced breakpoint. This + * obviously only works for breakpoints inserted with a reference. The + * lifetime of this objected will be limited and should not be kept + * beyond its immediate use. Otherwise return NULL. + */ +CPUBreakpoint *cpu_breakpoint_get_by_ref(CPUState *cpu, int ref); + +/** + * cpu_breakpoint_remove_by_ref: + * + * @cpu: CPU to monitor + * @ref: unique reference + * + * Remove a referenced breakpoint + */ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags); -void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint); +void cpu_breakpoint_remove_by_ref(CPUState *cpu, int ref); + void cpu_breakpoint_remove_all(CPUState *cpu, int mask); /* Return true if PC matches an installed breakpoint. */ diff --git a/linux-user/main.c b/linux-user/main.c index 2d65508..8f71608 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3816,7 +3816,7 @@ CPUArchState *cpu_copy(CPUArchState *env) int i; for (i = 0; i < cpu->breakpoints->len; i++) { bp = g_array_index(cpu->breakpoints, CPUBreakpoint *, i); - cpu_breakpoint_insert(new_cpu, bp->pc, bp->flags, NULL); + cpu_breakpoint_insert(new_cpu, bp->pc, bp->flags); } } if (unlikely(cpu->watchpoints) && unlikely(cpu->watchpoints->len)) { diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 4d04a76..279ae42 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -493,8 +493,6 @@ typedef struct CPUARMState { int eabi; #endif - struct CPUBreakpoint *cpu_breakpoint[16]; - CPU_COMMON /* These fields after the common ones so they are preserved on reset. */ diff --git a/target-arm/helper.c b/target-arm/helper.c index 50d70ce..3c751a4 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -4061,12 +4061,8 @@ void hw_breakpoint_update(ARMCPU *cpu, int n) int bt; int flags = BP_CPU; - if (env->cpu_breakpoint[n]) { - cpu_breakpoint_remove_by_ref(CPU(cpu), env->cpu_breakpoint[n]); - env->cpu_breakpoint[n] = NULL; - } - if (!extract64(bcr, 0, 1)) { + cpu_breakpoint_remove_by_ref(CPU(cpu), n); /* E bit clear : watchpoint disabled */ return; } @@ -4125,21 +4121,19 @@ void hw_breakpoint_update(ARMCPU *cpu, int n) return; } - cpu_breakpoint_insert(CPU(cpu), addr, flags, &env->cpu_breakpoint[n]); + cpu_breakpoint_insert_with_ref(CPU(cpu), addr, flags, n); } void hw_breakpoint_update_all(ARMCPU *cpu) { int i; - CPUARMState *env = &cpu->env; /* Completely clear out existing QEMU breakpoints and our array, to * avoid possible stale entries following migration load. */ cpu_breakpoint_remove_all(CPU(cpu), BP_CPU); - memset(env->cpu_breakpoint, 0, sizeof(env->cpu_breakpoint)); - for (i = 0; i < ARRAY_SIZE(cpu->env.cpu_breakpoint); i++) { + for (i = 0; i < 16; i++) { hw_breakpoint_update(cpu, i); } } diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 02d6265..2a24720 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -1071,8 +1071,8 @@ static bool bp_wp_matches(ARMCPU *cpu, int n, bool is_wp) } } else { uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; - - if (!env->cpu_breakpoint[n] || env->cpu_breakpoint[n]->pc != pc) { + CPUBreakpoint *bp = cpu_breakpoint_get_by_ref(CPU(cpu), n); + if (!bp || bp->pc != pc) { return false; } cr = env->cp15.dbgbcr[n]; @@ -1174,7 +1174,7 @@ static bool check_breakpoints(ARMCPU *cpu) return false; } - for (n = 0; n < ARRAY_SIZE(env->cpu_breakpoint); n++) { + for (n = 0; n < 16; n++) { if (bp_wp_matches(cpu, n, false)) { return true; } diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c index 1793e17..4f2c75f 100644 --- a/target-i386/bpt_helper.c +++ b/target-i386/bpt_helper.c @@ -56,13 +56,11 @@ static int hw_breakpoint_insert(CPUX86State *env, int index) CPUState *cs = CPU(x86_env_get_cpu(env)); target_ulong dr7 = env->dr[7]; target_ulong drN = env->dr[index]; - int err = 0; switch (hw_breakpoint_type(dr7, index)) { case DR7_TYPE_BP_INST: if (hw_breakpoint_enabled(dr7, index)) { - err = cpu_breakpoint_insert(cs, drN, BP_CPU, - &env->cpu_breakpoint[index]); + cpu_breakpoint_insert_with_ref(cs, drN, BP_CPU, index); } break; @@ -73,23 +71,21 @@ static int hw_breakpoint_insert(CPUX86State *env, int index) case DR7_TYPE_DATA_WR: if (hw_breakpoint_enabled(dr7, index)) { - err = cpu_watchpoint_insert_with_ref(cs, drN, - hw_breakpoint_len(dr7, index), - BP_CPU | BP_MEM_WRITE, index); + cpu_watchpoint_insert_with_ref(cs, drN, + hw_breakpoint_len(dr7, index), + BP_CPU | BP_MEM_WRITE, index); } break; case DR7_TYPE_DATA_RW: if (hw_breakpoint_enabled(dr7, index)) { - err = cpu_watchpoint_insert_with_ref(cs, drN, - hw_breakpoint_len(dr7, index), - BP_CPU | BP_MEM_ACCESS, index); + cpu_watchpoint_insert_with_ref(cs, drN, + hw_breakpoint_len(dr7, index), + BP_CPU | BP_MEM_ACCESS, index); } break; } - if (err) { - env->cpu_breakpoint[index] = NULL; - } + return 0; } @@ -99,10 +95,7 @@ static void hw_breakpoint_remove(CPUX86State *env, int index) switch (hw_breakpoint_type(env->dr[7], index)) { case DR7_TYPE_BP_INST: - if (env->cpu_breakpoint[index]) { - cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]); - env->cpu_breakpoint[index] = NULL; - } + cpu_breakpoint_remove_by_ref(cs, index); break; case DR7_TYPE_DATA_WR: diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 87f62be..fd5eed3 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1057,9 +1057,6 @@ typedef struct CPUX86State { int exception_is_int; target_ulong exception_next_eip; target_ulong dr[8]; /* debug registers; note dr4 and dr5 are unused */ - union { - struct CPUBreakpoint *cpu_breakpoint[4]; - }; /* breakpoints for dr[0..3] */ int old_exception; /* exception in flight */ uint64_t vm_vmcb; diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h index 1512119..8a64acd 100644 --- a/target-lm32/cpu.h +++ b/target-lm32/cpu.h @@ -162,8 +162,6 @@ struct CPULM32State { uint32_t bp[4]; /* breakpoints */ uint32_t wp[4]; /* watchpoints */ - struct CPUBreakpoint *cpu_breakpoint[4]; - CPU_COMMON /* Fields from here on are preserved across CPU reset. */ diff --git a/target-lm32/helper.c b/target-lm32/helper.c index 8c6ae52..029fbc5 100644 --- a/target-lm32/helper.c +++ b/target-lm32/helper.c @@ -60,20 +60,14 @@ void lm32_breakpoint_insert(CPULM32State *env, int idx, target_ulong address) { LM32CPU *cpu = lm32_env_get_cpu(env); - cpu_breakpoint_insert(CPU(cpu), address, BP_CPU, - &env->cpu_breakpoint[idx]); + cpu_breakpoint_insert_with_ref(CPU(cpu), address, BP_CPU, idx); } void lm32_breakpoint_remove(CPULM32State *env, int idx) { LM32CPU *cpu = lm32_env_get_cpu(env); - if (!env->cpu_breakpoint[idx]) { - return; - } - - cpu_breakpoint_remove_by_ref(CPU(cpu), env->cpu_breakpoint[idx]); - env->cpu_breakpoint[idx] = NULL; + cpu_breakpoint_remove_by_ref(CPU(cpu), idx); } void lm32_watchpoint_insert(CPULM32State *env, int idx, target_ulong address, -- 2.7.4