[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 07/23] exec.c: Relax restrictions on watchpoint lengt
From: |
Peter Maydell |
Subject: |
[Qemu-devel] [PULL 07/23] exec.c: Relax restrictions on watchpoint length and alignment |
Date: |
Fri, 12 Sep 2014 14:23:38 +0100 |
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 <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
---
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 7dddcc8..f3e7fb6 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 1aafbf5..7c06f37 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 472a16d..483eb3f 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
--
1.9.1
- [Qemu-devel] [PULL 09/23] exec.c: Record watchpoint fault address and direction, (continued)
- [Qemu-devel] [PULL 09/23] exec.c: Record watchpoint fault address and direction, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 18/23] target-arm: Make *IS TLB maintenance ops affect all CPUs, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 23/23] hw/arm/boot: enable DTB support when booting ELF images, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 11/23] target-arm: Implement setting of watchpoints, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 14/23] target-arm: Set DBGDSCR.MOE for debug exceptions taken to AArch32, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 10/23] cpu-exec: Make debug_excp_handler a QOM CPU method, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 16/23] target-arm: Implement minimal DBGVCR, OSDLR_EL1, MDCCSR_EL0, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 15/23] target-arm: Remove comment about MDSCR_EL1 being dummy implementation, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 19/23] hw/arm/virt: fix pl011 and pl031 irq flags, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 05/23] target-arm: Fix broken indentation in arm_cpu_reest(), Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 07/23] exec.c: Relax restrictions on watchpoint length and alignment,
Peter Maydell <=
- [Qemu-devel] [PULL 06/23] hw/arm/virt: Provide flash devices for boot ROMs, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 12/23] target-arm: Move extended_addresses_enabled() to internals.h, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 20/23] hw/arm/boot: load DTB as a ROM image, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 01/23] hw/arm/virt: add linux, stdout-path to /chosen DT node, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 08/23] exec.c: Provide full set of dummy wp remove functions in user-mode, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 21/23] hw/arm/boot: pass an address limit to and return size from load_dtb(), Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 03/23] pl061: implement input interrupt logic, Peter Maydell, 2014/09/12
- [Qemu-devel] [PULL 04/23] target-arm: Fix resetting issues on ARMv7-M CPUs, Peter Maydell, 2014/09/12