From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KpobW-0003WD-VS for qemu-devel@nongnu.org; Tue, 14 Oct 2008 14:26:47 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KpobV-0003VE-PD for qemu-devel@nongnu.org; Tue, 14 Oct 2008 14:26:45 -0400 Received: from [199.232.76.173] (port=52862 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KpobV-0003Uw-A9 for qemu-devel@nongnu.org; Tue, 14 Oct 2008 14:26:45 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:36376) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KpobU-0000lT-KY for qemu-devel@nongnu.org; Tue, 14 Oct 2008 14:26:45 -0400 Received: from smtp06.web.de (fmsmtp06.dlan.cinetic.de [172.20.5.172]) by fmmailgate02.web.de (Postfix) with ESMTP id C127EF331B4B for ; Tue, 14 Oct 2008 20:26:43 +0200 (CEST) Received: from [88.64.28.10] (helo=[192.168.1.198]) by smtp06.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.109 #226) id 1KpobT-0005Wd-00 for qemu-devel@nongnu.org; Tue, 14 Oct 2008 20:26:43 +0200 Message-ID: <48F4E45F.1060300@web.de> Date: Tue, 14 Oct 2008 20:26:39 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20081014091223.932366369@mchn012c.ww002.siemens.net> <20081014091225.115644848@mchn012c.ww002.siemens.net> <5d6222a80810141050p1de281e1r7738434b7127d638@mail.gmail.com> In-Reply-To: <5d6222a80810141050p1de281e1r7738434b7127d638@mail.gmail.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig6B9F7A34B30520E3B06ADAF2" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH 04/13] Respect length of watchpoints Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig6B9F7A34B30520E3B06ADAF2 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Glauber Costa wrote: > On Tue, Oct 14, 2008 at 7:12 AM, Jan Kiszka wr= ote: >> This adds length support for watchpoints. To keep things simple, only >> aligned watchpoints are accepted. >=20 > why? It does not seem that much complicated to handle unaligned watchpo= ints. > Unless I'm totally wrong, we should just store the value as-is, and > then check for it. > As a matter of fact, because we're masking and testing for the mask, > it seems even more > complicated to require that. I agree a full aligned world would be a > happier world, but unfortunately, > unaligned accesses are quite common in x86. Unaligned watchpoints also means multi-page watchpoints - and this introduces some complexity. I think the fact that real x86 hw watchpoints require alignment as well motivated the simplification. But if there is a real need for it (e.g. some other arch using the infrastructure for hw watchpoint emulation), I could rethink this. However, I would prefer to apply such extension on top of the proposed implementation. >=20 >> Signed-off-by: Jan Kiszka >> --- >> cpu-defs.h | 2 +- >> exec.c | 30 ++++++++++++++++++++---------- >> 2 files changed, 21 insertions(+), 11 deletions(-) >> >> Index: b/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/exec.c >> +++ b/exec.c >> @@ -1313,14 +1313,21 @@ static void breakpoint_invalidate(CPUSta >> int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ul= ong len, >> int flags, CPUWatchpoint **watchpoint) >> { >> + target_ulong len_mask =3D ~(len - 1); >> CPUWatchpoint *wp; >> >> + /* sanity checks: allow power-of-2 lengths, deny unaligned watchp= oints */ >> + if ((len !=3D 1 && len !=3D 2 && len !=3D 4 && len !=3D 8) || (ad= dr & ~len_mask)) { >> + fprintf(stderr, "qemu: tried to set invalid watchpoint at " >> + TARGET_FMT_lx ", len=3D" TARGET_FMT_lu "\n", addr, le= n); >> + return -EINVAL; >> + } >> wp =3D qemu_malloc(sizeof(*wp)); >> if (!wp) >> return -ENOBUFS; >> >> wp->vaddr =3D addr; >> - wp->len =3D len; >> + wp->len_mask =3D len_mask; >> wp->flags =3D flags; >> >> wp->next =3D env->watchpoints; >> @@ -1344,10 +1351,12 @@ int cpu_watchpoint_insert(CPUState *env, >> int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ul= ong len, >> int flags) >> { >> + target_ulong len_mask =3D ~(len - 1); >> CPUWatchpoint *wp; >> >> 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; >> } >> @@ -2502,7 +2511,7 @@ static CPUWriteMemoryFunc *notdirty_mem_ >> }; >> >> /* 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; >> @@ -2510,7 +2519,8 @@ static void check_watchpoint(int offset, >> >> 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 &= flags)) { >> env->watchpoint_hit =3D wp; >> cpu_interrupt(env, CPU_INTERRUPT_DEBUG); >> break; >> @@ -2523,40 +2533,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); >> } >> >> 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); >> } >> >> 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); >> } >> >> 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); >> } >> >> 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); >> } >> >> 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); >> } >> >> Index: b/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/cpu-defs.h >> +++ b/cpu-defs.h >> @@ -148,7 +148,7 @@ typedef struct CPUBreakpoint { >> >> typedef struct CPUWatchpoint { >> target_ulong vaddr; >> - target_ulong len; >> + target_ulong len_mask; >> int flags; /* BP_* */ >> struct CPUWatchpoint *prev, *next; >> } CPUWatchpoint; >=20 > It's less confusing if you call it len_mask from the beginning, > instead of changing your own patch for that purpose. OK. If I have to update the involved patches, I will merge this over. Jan --------------enig6B9F7A34B30520E3B06ADAF2 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 iEYEARECAAYFAkj05GMACgkQniDOoMHTA+kzGwCdEEOWHivhYEqplS6RUR7P8tOs roIAn2b7NhYA+B2tiD7OLedOlVLIm9+X =8tBn -----END PGP SIGNATURE----- --------------enig6B9F7A34B30520E3B06ADAF2--