There is no particular reason why you can't have a watchpoint in TCG
that covers a large chunk of the address space. We could be clever
about it but these cases are pretty rare and we can assume the user
will expect a little performance degradation.
NB: In my testing gdb will silently squash a watchpoint like:
watch (char[0x7fffffffff]) *0x0
to a 4 byte watchpoint. Practically it will limit the maximum size
based on max-value-size. However given enough of a tweak the sky is
the limit.
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- use cleaner in_page = -(addr | TARGET_PAGE_MASK) logic per rth
---
exec.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/exec.c b/exec.c
index 5162f0d12f9..65a4376df37 100644
--- a/exec.c
+++ b/exec.c
@@ -1036,6 +1036,7 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
int flags, CPUWatchpoint **watchpoint)
{
CPUWatchpoint *wp;
+ vaddr in_page;
/* forbid ranges which are empty or run off the end of the address space */
if (len == 0 || (addr + len - 1) < addr) {
@@ -1056,7 +1057,12 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry);
}
- tlb_flush_page(cpu, addr);
+ in_page = -(addr | TARGET_PAGE_MASK);
+ if (len <= in_page) {
+ tlb_flush_page(cpu, addr);
+ } else {
+ tlb_flush(cpu);
+ }
if (watchpoint)
*watchpoint = wp;
--
2.20.1
On 6/3/20 1:24 PM, Alex Bennée wrote: > There is no particular reason why you can't have a watchpoint in TCG > that covers a large chunk of the address space. We could be clever > about it but these cases are pretty rare and we can assume the user > will expect a little performance degradation. > > NB: In my testing gdb will silently squash a watchpoint like: > > watch (char[0x7fffffffff]) *0x0 > > to a 4 byte watchpoint. Practically it will limit the maximum size > based on max-value-size. However given enough of a tweak the sky is > the limit. > > Reported-by: Alexander Bulekov <alxndr@bu.edu> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > --- > v2 > - use cleaner in_page = -(addr | TARGET_PAGE_MASK) logic per rth Can we have a macro for this? Maybe QEMU_IN_PAGE_OFFSET(addr, TARGET_PAGE_MASK)? or QEMU_OFFSET_IN_PAGE()... > --- > exec.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 5162f0d12f9..65a4376df37 100644 > --- a/exec.c > +++ b/exec.c > @@ -1036,6 +1036,7 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, > int flags, CPUWatchpoint **watchpoint) > { > CPUWatchpoint *wp; > + vaddr in_page; > > /* forbid ranges which are empty or run off the end of the address space */ > if (len == 0 || (addr + len - 1) < addr) { > @@ -1056,7 +1057,12 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, > QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry); > } > > - tlb_flush_page(cpu, addr); > + in_page = -(addr | TARGET_PAGE_MASK); > + if (len <= in_page) { > + tlb_flush_page(cpu, addr); > + } else { > + tlb_flush(cpu); > + } > > if (watchpoint) > *watchpoint = wp; >
On 6/3/20 2:46 PM, Philippe Mathieu-Daudé wrote: > On 6/3/20 1:24 PM, Alex Bennée wrote: >> There is no particular reason why you can't have a watchpoint in TCG >> that covers a large chunk of the address space. We could be clever >> about it but these cases are pretty rare and we can assume the user >> will expect a little performance degradation. >> >> NB: In my testing gdb will silently squash a watchpoint like: >> >> watch (char[0x7fffffffff]) *0x0 >> >> to a 4 byte watchpoint. Practically it will limit the maximum size >> based on max-value-size. However given enough of a tweak the sky is >> the limit. >> >> Reported-by: Alexander Bulekov <alxndr@bu.edu> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> >> --- >> v2 >> - use cleaner in_page = -(addr | TARGET_PAGE_MASK) logic per rth > > Can we have a macro for this? > Maybe QEMU_IN_PAGE_OFFSET(addr, TARGET_PAGE_MASK)? > or QEMU_OFFSET_IN_PAGE()... As this is queued, I suppose the implicit answer is "no." > >> --- >> exec.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/exec.c b/exec.c >> index 5162f0d12f9..65a4376df37 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1036,6 +1036,7 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, >> int flags, CPUWatchpoint **watchpoint) >> { >> CPUWatchpoint *wp; >> + vaddr in_page; >> >> /* forbid ranges which are empty or run off the end of the address space */ >> if (len == 0 || (addr + len - 1) < addr) { >> @@ -1056,7 +1057,12 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, >> QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry); >> } >> >> - tlb_flush_page(cpu, addr); >> + in_page = -(addr | TARGET_PAGE_MASK); >> + if (len <= in_page) { >> + tlb_flush_page(cpu, addr); >> + } else { >> + tlb_flush(cpu); >> + } >> >> if (watchpoint) >> *watchpoint = wp; >> >
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > On 6/3/20 2:46 PM, Philippe Mathieu-Daudé wrote: >> On 6/3/20 1:24 PM, Alex Bennée wrote: >>> There is no particular reason why you can't have a watchpoint in TCG >>> that covers a large chunk of the address space. We could be clever >>> about it but these cases are pretty rare and we can assume the user >>> will expect a little performance degradation. >>> >>> NB: In my testing gdb will silently squash a watchpoint like: >>> >>> watch (char[0x7fffffffff]) *0x0 >>> >>> to a 4 byte watchpoint. Practically it will limit the maximum size >>> based on max-value-size. However given enough of a tweak the sky is >>> the limit. >>> >>> Reported-by: Alexander Bulekov <alxndr@bu.edu> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> >>> --- >>> v2 >>> - use cleaner in_page = -(addr | TARGET_PAGE_MASK) logic per rth >> >> Can we have a macro for this? >> Maybe QEMU_IN_PAGE_OFFSET(addr, TARGET_PAGE_MASK)? >> or QEMU_OFFSET_IN_PAGE()... > > As this is queued, I suppose the implicit answer is "no." Richard took it into tcg/next as is. I think having a macro may well be nice clean-up but I struggled to pick a good include location so left it for a future clean-up series ;-) -- Alex Bennée
On 6/3/20 4:24 AM, Alex Bennée wrote: > There is no particular reason why you can't have a watchpoint in TCG > that covers a large chunk of the address space. We could be clever > about it but these cases are pretty rare and we can assume the user > will expect a little performance degradation. > > NB: In my testing gdb will silently squash a watchpoint like: > > watch (char[0x7fffffffff]) *0x0 > > to a 4 byte watchpoint. Practically it will limit the maximum size > based on max-value-size. However given enough of a tweak the sky is > the limit. > > Reported-by: Alexander Bulekov <alxndr@bu.edu> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > --- > v2 > - use cleaner in_page = -(addr | TARGET_PAGE_MASK) logic per rth > --- > exec.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Queued to tcg-next. r~
© 2016 - 2024 Red Hat, Inc.