From 05068c0dfb5b23dde42ad0112123bdc8408a1f44 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 12 Sep 2014 14:06:48 +0100 Subject: [PATCH] exec.c: Relax restrictions on watchpoint length and alignment The current implementation of watchpoints requires that they have a power of 2 length which is not greater than TARGET_PAGE_SIZE and that their address is a multiple of their length. Watchpoints on ARM don't fit these restrictions, so change the implementation so they can be relaxed. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- exec.c | 44 +++++++++++++++++++++++++++++++------------- include/qom/cpu.h | 2 +- linux-user/main.c | 3 +-- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/exec.c b/exec.c index 7dddcc8034..f3e7fb6bcf 100644 --- a/exec.c +++ b/exec.c @@ -582,12 +582,10 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, int flags, CPUWatchpoint **watchpoint) { - vaddr len_mask = ~(len - 1); CPUWatchpoint *wp; - /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoints */ - if ((len & (len - 1)) || (addr & ~len_mask) || - len == 0 || len > TARGET_PAGE_SIZE) { + /* forbid ranges which are empty or run off the end of the address space */ + if (len == 0 || (addr + len - 1) <= addr) { error_report("tried to set invalid watchpoint at %" VADDR_PRIx ", len=%" VADDR_PRIu, addr, len); return -EINVAL; @@ -595,7 +593,7 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, wp = g_malloc(sizeof(*wp)); wp->vaddr = addr; - wp->len_mask = len_mask; + wp->len = len; wp->flags = flags; /* keep all GDB-injected watchpoints in front */ @@ -616,11 +614,10 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len, int flags) { - vaddr len_mask = ~(len - 1); CPUWatchpoint *wp; QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { - if (addr == wp->vaddr && len_mask == wp->len_mask + if (addr == wp->vaddr && len == wp->len && flags == (wp->flags & ~BP_WATCHPOINT_HIT)) { cpu_watchpoint_remove_by_ref(cpu, wp); return 0; @@ -650,6 +647,27 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int mask) } } } + +/* Return true if this watchpoint address matches the specified + * access (ie the address range covered by the watchpoint overlaps + * partially or completely with the address range covered by the + * access). + */ +static inline bool cpu_watchpoint_address_matches(CPUWatchpoint *wp, + vaddr addr, + vaddr len) +{ + /* We know the lengths are non-zero, but a little caution is + * required to avoid errors in the case where the range ends + * exactly at the top of the address space and so addr + len + * wraps round to zero. + */ + vaddr wpend = wp->vaddr + wp->len - 1; + vaddr addrend = addr + len - 1; + + return !(addr > wpend || wp->vaddr > addrend); +} + #endif /* Add a breakpoint. */ @@ -861,7 +879,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu, /* Make accesses to pages with watchpoints go via the watchpoint trap routines. */ QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { - if (vaddr == (wp->vaddr & TARGET_PAGE_MASK)) { + if (cpu_watchpoint_address_matches(wp, vaddr, TARGET_PAGE_SIZE)) { /* Avoid trapping reads of pages with a write breakpoint. */ if ((prot & PAGE_WRITE) || (wp->flags & BP_MEM_READ)) { iotlb = PHYS_SECTION_WATCH + paddr; @@ -1625,7 +1643,7 @@ static const MemoryRegionOps notdirty_mem_ops = { }; /* Generate a debug exception if a watchpoint has been hit. */ -static void check_watchpoint(int offset, int len_mask, int flags) +static void check_watchpoint(int offset, int len, int flags) { CPUState *cpu = current_cpu; CPUArchState *env = cpu->env_ptr; @@ -1643,8 +1661,8 @@ static void check_watchpoint(int offset, int len_mask, int flags) } vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { - if ((vaddr == (wp->vaddr & len_mask) || - (vaddr & wp->len_mask) == wp->vaddr) && (wp->flags & flags)) { + if (cpu_watchpoint_address_matches(wp, vaddr, len) + && (wp->flags & flags)) { wp->flags |= BP_WATCHPOINT_HIT; if (!cpu->watchpoint_hit) { cpu->watchpoint_hit = wp; @@ -1670,7 +1688,7 @@ static void check_watchpoint(int offset, int len_mask, int flags) static uint64_t watch_mem_read(void *opaque, hwaddr addr, unsigned size) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, ~(size - 1), BP_MEM_READ); + check_watchpoint(addr & ~TARGET_PAGE_MASK, size, BP_MEM_READ); switch (size) { case 1: return ldub_phys(&address_space_memory, addr); case 2: return lduw_phys(&address_space_memory, addr); @@ -1682,7 +1700,7 @@ static uint64_t watch_mem_read(void *opaque, hwaddr addr, static void watch_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, ~(size - 1), BP_MEM_WRITE); + check_watchpoint(addr & ~TARGET_PAGE_MASK, size, BP_MEM_WRITE); switch (size) { case 1: stb_phys(&address_space_memory, addr, val); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 1aafbf5f34..7c06f3711a 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -169,7 +169,7 @@ typedef struct CPUBreakpoint { typedef struct CPUWatchpoint { vaddr vaddr; - vaddr len_mask; + vaddr len; int flags; /* BP_* */ QTAILQ_ENTRY(CPUWatchpoint) entry; } CPUWatchpoint; diff --git a/linux-user/main.c b/linux-user/main.c index 472a16d2db..483eb3fec2 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3458,8 +3458,7 @@ CPUArchState *cpu_copy(CPUArchState *env) cpu_breakpoint_insert(new_cpu, bp->pc, bp->flags, NULL); } QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { - cpu_watchpoint_insert(new_cpu, wp->vaddr, (~wp->len_mask) + 1, - wp->flags, NULL); + cpu_watchpoint_insert(new_cpu, wp->vaddr, wp->len, wp->flags, NULL); } #endif