| Message ID | 20200311064420.30606-4-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: sve load/store improvements | expand |
On Wed, 11 Mar 2020 at 06:44, Richard Henderson <richard.henderson@linaro.org> wrote: > > Allow probing of a watchpoint *without* raising an exception. > This is of most use for no-fault loads, which should indicate > via some architectural means that the load did not occur. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/hw/core/cpu.h | 7 +++++++ > exec.c | 19 +++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index 73e9a869a4..8ec44267a4 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -1090,6 +1090,12 @@ static inline void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, > { > } > > +static inline bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, > + int flags) > +{ > + return false; > +} > + > static inline int cpu_watchpoint_address_matches(CPUState *cpu, > vaddr addr, vaddr len) > { > @@ -1104,6 +1110,7 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint); > void cpu_watchpoint_remove_all(CPUState *cpu, int mask); > void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, > MemTxAttrs attrs, int flags, uintptr_t ra); > +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags); Could we have a doc comment for the new function? > int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len); > #endif > > diff --git a/exec.c b/exec.c > index 0cc500d53a..2b8f601b9e 100644 > --- a/exec.c > +++ b/exec.c > @@ -2735,6 +2735,25 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, > } > } > > +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags) > +{ > + CPUClass *cc = CPU_GET_CLASS(cpu); > + CPUWatchpoint *wp; > + > + assert(tcg_enabled()); > + > + addr = cc->adjust_watchpoint_address(cpu, addr, len); > + QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { > + if (watchpoint_address_matches(wp, addr, len) && > + (wp->flags & flags) && > + (!(wp->flags & BP_CPU) || > + !cc->debug_check_watchpoint(cpu, wp))) { > + return true; > + } > + } > + return false; > +} Clearly the insn emulation needs to do the right thing for guest architectural watchpoints, but should a gdb watchpoint also affect no-fault-load behaviour? I suppose making them both behave the same way is probably the least-surprising choice. thanks -- PMM
On 4/16/20 5:08 AM, Peter Maydell wrote: >> void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, >> MemTxAttrs attrs, int flags, uintptr_t ra); >> +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags); > > Could we have a doc comment for the new function? > >> int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len); Hah. In the process of doing that, I notice that cpu_watchpoint_address_matches actually does what I want. I have added documentation for cpu_check_watchpoint and cpu_watchpoint_address_matches and have dropped this new function. > Clearly the insn emulation needs to do the right thing for > guest architectural watchpoints, but should a gdb watchpoint > also affect no-fault-load behaviour? I suppose making them > both behave the same way is probably the least-surprising choice. In both cases we need to interrupt the execution in order to actually honor the watchpoint. So yes, treating them the same seems the only reasonable way. r~
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 73e9a869a4..8ec44267a4 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1090,6 +1090,12 @@ static inline void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, { } +static inline bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, + int flags) +{ + return false; +} + static inline int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len) { @@ -1104,6 +1110,7 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint); void cpu_watchpoint_remove_all(CPUState *cpu, int mask); void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, MemTxAttrs attrs, int flags, uintptr_t ra); +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags); int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len); #endif diff --git a/exec.c b/exec.c index 0cc500d53a..2b8f601b9e 100644 --- a/exec.c +++ b/exec.c @@ -2735,6 +2735,25 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, } } +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags) +{ + CPUClass *cc = CPU_GET_CLASS(cpu); + CPUWatchpoint *wp; + + assert(tcg_enabled()); + + addr = cc->adjust_watchpoint_address(cpu, addr, len); + QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { + if (watchpoint_address_matches(wp, addr, len) && + (wp->flags & flags) && + (!(wp->flags & BP_CPU) || + !cc->debug_check_watchpoint(cpu, wp))) { + return true; + } + } + return false; +} + static MemTxResult flatview_read(FlatView *fv, hwaddr addr, MemTxAttrs attrs, void *buf, hwaddr len); static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
Allow probing of a watchpoint *without* raising an exception. This is of most use for no-fault loads, which should indicate via some architectural means that the load did not occur. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/hw/core/cpu.h | 7 +++++++ exec.c | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) -- 2.20.1