diff mbox series

[RFC] exec: flush the whole TLB if a watchpoint crosses a page boundary

Message ID 20200602164911.5706-1-alex.bennee@linaro.org
State New
Headers show
Series [RFC] exec: flush the whole TLB if a watchpoint crosses a page boundary | expand

Commit Message

Alex Bennée June 2, 2020, 4:49 p.m. UTC
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>

---
 exec.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.20.1

Comments

Richard Henderson June 2, 2020, 5:12 p.m. UTC | #1
On 6/2/20 9:49 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>

> ---

>  exec.c | 6 +++++-

>  1 file changed, 5 insertions(+), 1 deletion(-)

> 

> diff --git a/exec.c b/exec.c

> index 5162f0d12f9..851ac180fe7 100644

> --- a/exec.c

> +++ b/exec.c

> @@ -1056,7 +1056,11 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,

>          QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry);

>      }

>  

> -    tlb_flush_page(cpu, addr);

> +    if (((addr + len) & TARGET_PAGE_MASK) != (addr & TARGET_PAGE_MASK)) {


This computation will be false when adding a watchpoint for the last len bytes
in the page, and no actual page crossing is required.  For this reason I prefer

    in_page = -(addr | TARGET_PAGE_MASK);
    if (len <= in_page)


r~

> +        tlb_flush(cpu);

> +    } else {

> +        tlb_flush_page(cpu, addr);

> +    }

>  

>      if (watchpoint)

>          *watchpoint = wp;

>
diff mbox series

Patch

diff --git a/exec.c b/exec.c
index 5162f0d12f9..851ac180fe7 100644
--- a/exec.c
+++ b/exec.c
@@ -1056,7 +1056,11 @@  int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
         QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry);
     }
 
-    tlb_flush_page(cpu, addr);
+    if (((addr + len) & TARGET_PAGE_MASK) != (addr & TARGET_PAGE_MASK)) {
+        tlb_flush(cpu);
+    } else {
+        tlb_flush_page(cpu, addr);
+    }
 
     if (watchpoint)
         *watchpoint = wp;