From 05068c0dfb5b23dde42ad0112123bdc8408a1f44 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 12 Sep 2014 14:06:48 +0100 Subject: 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 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) (limited to 'exec.c') 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); -- cgit 1.4.1 From 3ee887e8ff7610d83bf05b0ebd5a1d891f0d8816 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 12 Sep 2014 14:06:48 +0100 Subject: exec.c: Provide full set of dummy wp remove functions in user-mode We already provide dummy versions of the cpu_watchpoint_insert and cpu_watchpoint_remove_all functions when CONFIG_USER_ONLY is defined. Complete the set by providing cpu_watchpoint_remove and cpu_watchpoint_remove_by_ref as well. This allows target-* code using these functions to avoid some ifdeffery. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- exec.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'exec.c') diff --git a/exec.c b/exec.c index f3e7fb6bcf..181ade0298 100644 --- a/exec.c +++ b/exec.c @@ -572,6 +572,16 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int mask) { } +int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len, + int flags) +{ + return -ENOSYS; +} + +void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint) +{ +} + int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len, int flags, CPUWatchpoint **watchpoint) { -- cgit 1.4.1 From 08225676b279fd14683275b65ed701972e008043 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 12 Sep 2014 14:06:48 +0100 Subject: exec.c: Record watchpoint fault address and direction When we check whether we've hit a watchpoint we know the address that we were attempting to access and whether it was a read or a write. Record this information in the CPUWatchpoint struct so that target-specific code can report it to the guest. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- exec.c | 7 ++++++- include/qom/cpu.h | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'exec.c') diff --git a/exec.c b/exec.c index 181ade0298..2794b4ba23 100644 --- a/exec.c +++ b/exec.c @@ -1673,7 +1673,12 @@ static void check_watchpoint(int offset, int len, int flags) QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) { if (cpu_watchpoint_address_matches(wp, vaddr, len) && (wp->flags & flags)) { - wp->flags |= BP_WATCHPOINT_HIT; + if (flags == BP_MEM_READ) { + wp->flags |= BP_WATCHPOINT_HIT_READ; + } else { + wp->flags |= BP_WATCHPOINT_HIT_WRITE; + } + wp->hitaddr = vaddr; if (!cpu->watchpoint_hit) { cpu->watchpoint_hit = wp; tb_check_watchpoint(cpu); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 7c06f3711a..c325774a3c 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -170,6 +170,7 @@ typedef struct CPUBreakpoint { typedef struct CPUWatchpoint { vaddr vaddr; vaddr len; + vaddr hitaddr; int flags; /* BP_* */ QTAILQ_ENTRY(CPUWatchpoint) entry; } CPUWatchpoint; @@ -622,9 +623,12 @@ void cpu_single_step(CPUState *cpu, int enabled); #define BP_MEM_WRITE 0x02 #define BP_MEM_ACCESS (BP_MEM_READ | BP_MEM_WRITE) #define BP_STOP_BEFORE_ACCESS 0x04 -#define BP_WATCHPOINT_HIT 0x08 +/* 0x08 currently unused */ #define BP_GDB 0x10 #define BP_CPU 0x20 +#define BP_WATCHPOINT_HIT_READ 0x40 +#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); -- cgit 1.4.1