From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 05/17] qemu: Respect length of watchpoints Date: Wed, 08 Oct 2008 22:22:34 +0200 Message-ID: <48ED168A.2080301@web.de> References: <20081006091415.095241851@mchn012c.ww002.siemens.net> <20081006091416.087984430@mchn012c.ww002.siemens.net> <48EB5040.9090703@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig462B59BBBC445E1DA5095095" Cc: kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:44197 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754522AbYJHUWy (ORCPT ); Wed, 8 Oct 2008 16:22:54 -0400 In-Reply-To: <48EB5040.9090703@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig462B59BBBC445E1DA5095095 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Avi Kivity wrote: > Jan Kiszka wrote: >> This adds length support for watchpoints. To keep things simple, only >> aligned watchpoints are accepted. >> >> --- a/qemu/exec.c >> +++ b/qemu/exec.c >> @@ -1328,14 +1328,19 @@ static void breakpoint_invalidate(CPUSta >> int cpu_watchpoint_insert(CPUState *env, target_ulong addr, >> target_ulong len, >> int flags, CPUWatchpoint **watchpoint) >> { >> + target_ulong len_mask =3D ~(len - 1); >> CPUWatchpoint *wp; >> =20 >> + /* sanity checks: allow power-of-2 lengths, deny unaligned >> watchpoints */ >> + if ((len !=3D 1 && len !=3D 2 && len !=3D 4) || (addr & ~len_mask= )) >> + return -EINVAL; >> + >> =20 >=20 > It would be good to support 8-byte watchpoints (as x86-64 does); also, > print a message if we deny a breakpoint due to bad alignment, so people= > know where to fix this. >=20 Yep, here the updated patch. [ This, as well as the rest, for the convenience of testers under KVM. Will repost the updated QEMU series next week. ] ------------ This adds length support for watchpoints. To keep things simple, only aligned watchpoints are accepted. Signed-off-by: Jan Kiszka --- qemu/cpu-defs.h | 2 +- qemu/exec.c | 30 ++++++++++++++++++++---------- 2 files changed, 21 insertions(+), 11 deletions(-) Index: b/qemu/exec.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- a/qemu/exec.c +++ b/qemu/exec.c @@ -1328,14 +1328,21 @@ static void breakpoint_invalidate(CPUSta int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong= len, int flags, CPUWatchpoint **watchpoint) { + target_ulong len_mask =3D ~(len - 1); CPUWatchpoint *wp; =20 + /* sanity checks: allow power-of-2 lengths, deny unaligned watchpoin= ts */ + if ((len !=3D 1 && len !=3D 2 && len !=3D 4 && len !=3D 8) || (addr = & ~len_mask)) { + fprintf(stderr, "qemu: tried to set invalid watchpoint at " + TARGET_FMT_lx ", len=3D" TARGET_FMT_lu "\n", addr, len);= + return -EINVAL; + } wp =3D qemu_malloc(sizeof(*wp)); if (!wp) return -ENOBUFS; =20 wp->vaddr =3D addr; - wp->len =3D len; + wp->len_mask =3D len_mask; wp->flags =3D flags; =20 wp->next =3D env->watchpoints; @@ -1359,10 +1366,12 @@ int cpu_watchpoint_insert(CPUState *env, int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong= len, int flags) { + target_ulong len_mask =3D ~(len - 1); CPUWatchpoint *wp; =20 for (wp =3D env->watchpoints; wp !=3D NULL; wp =3D wp->next) { - if (addr =3D=3D wp->vaddr && len =3D=3D wp->len && flags =3D=3D = wp->flags) { + if (addr =3D=3D wp->vaddr && len_mask =3D=3D wp->len_mask + && flags =3D=3D wp->flags) { cpu_watchpoint_remove_by_ref(env, wp); return 0; } @@ -2490,7 +2499,7 @@ static CPUWriteMemoryFunc *notdirty_mem_ }; =20 /* Generate a debug exception if a watchpoint has been hit. */ -static void check_watchpoint(int offset, int flags) +static void check_watchpoint(int offset, int len_mask, int flags) { CPUState *env =3D cpu_single_env; target_ulong vaddr; @@ -2498,7 +2507,8 @@ static void check_watchpoint(int offset, =20 vaddr =3D (env->mem_io_vaddr & TARGET_PAGE_MASK) + offset; for (wp =3D env->watchpoints; wp !=3D NULL; wp =3D wp->next) { - if (vaddr =3D=3D wp->vaddr && (wp->flags & flags)) { + if ((vaddr =3D=3D (wp->vaddr & len_mask) || + (vaddr & wp->len_mask) =3D=3D wp->vaddr) && (wp->flags & fl= ags)) { env->watchpoint_hit =3D wp; cpu_interrupt(env, CPU_INTERRUPT_DEBUG); break; @@ -2511,40 +2521,40 @@ static void check_watchpoint(int offset, phys routines. */ static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ); + check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x0, BP_MEM_READ); return ldub_phys(addr); } =20 static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ); + check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x1, BP_MEM_READ); return lduw_phys(addr); } =20 static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ); + check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x3, BP_MEM_READ); return ldl_phys(addr); } =20 static void watch_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE); + check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x0, BP_MEM_WRITE); stb_phys(addr, val); } =20 static void watch_mem_writew(void *opaque, target_phys_addr_t addr, uint32_t val) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE); + check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x1, BP_MEM_WRITE); stw_phys(addr, val); } =20 static void watch_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val) { - check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE); + check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x3, BP_MEM_WRITE); stl_phys(addr, val); } =20 Index: b/qemu/cpu-defs.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- a/qemu/cpu-defs.h +++ b/qemu/cpu-defs.h @@ -148,7 +148,7 @@ typedef struct CPUBreakpoint { =20 typedef struct CPUWatchpoint { target_ulong vaddr; - target_ulong len; + target_ulong len_mask; int flags; /* BP_* */ struct CPUWatchpoint *prev, *next; } CPUWatchpoint; --------------enig462B59BBBC445E1DA5095095 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkjtFpQACgkQniDOoMHTA+kyVwCdEjyOHIwM2/4mshBlNmGu/zNz t3sAn2qtjGXuoJUNTeAcizZYrCylGYVO =YC9i -----END PGP SIGNATURE----- --------------enig462B59BBBC445E1DA5095095--