From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60257) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2aHe-000785-SO for qemu-devel@nongnu.org; Fri, 04 Nov 2016 04:55:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2aHb-0000vV-Q6 for qemu-devel@nongnu.org; Fri, 04 Nov 2016 04:55:34 -0400 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:33453) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c2aHb-0000tc-Jm for qemu-devel@nongnu.org; Fri, 04 Nov 2016 04:55:31 -0400 Received: by mail-wm0-x244.google.com with SMTP id u144so2826039wmu.0 for ; Fri, 04 Nov 2016 01:55:31 -0700 (PDT) Sender: Paolo Bonzini References: <1478194258-75276-1-git-send-email-julian@codesourcery.com> <1478194258-75276-5-git-send-email-julian@codesourcery.com> <20161103232039.42e2ea11@squid.athome> From: Paolo Bonzini Message-ID: <9b6b35db-c7b1-dad3-980f-bcaa418baaaa@redhat.com> Date: Fri, 4 Nov 2016 09:55:17 +0100 MIME-Version: 1.0 In-Reply-To: <20161103232039.42e2ea11@squid.athome> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/5] ARM BE32 watchpoint fix. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Julian Brown , Peter Maydell Cc: QEMU Developers On 04/11/2016 00:20, Julian Brown wrote: > On Thu, 3 Nov 2016 23:14:05 +0000 > Peter Maydell wrote: > >> On 3 November 2016 at 17:30, Julian Brown >> wrote: >>> In BE32 mode, sub-word size watchpoints can fail to trigger because >>> the address of the access is adjusted in the opcode helpers before >>> being compared with the watchpoint registers. This patch reversed >>> the address adjustment before performing the comparison. >>> >>> Signed-off-by: Julian Brown >>> --- >>> exec.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/exec.c b/exec.c >>> index 4c84389..eadab54 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -2047,6 +2047,19 @@ static void check_watchpoint(int offset, int >>> len, MemTxAttrs attrs, int flags) return; >>> } >>> vaddr = (cpu->mem_io_vaddr & TARGET_PAGE_MASK) + offset; >>> +#if defined(TARGET_ARM) && !defined(CONFIG_USER_ONLY) >>> + /* In BE32 system mode, target memory is stored byteswapped >>> (FIXME: >>> + relative to a little-endian host system), and by the time >>> we reach here >>> + (via an opcode helper) the addresses of subword accesses >>> have been >>> + adjusted to account for that, which means that watchpoints >>> will not >>> + match. Undo the adjustment here. */ >>> + if (arm_sctlr_b(env)) { >>> + if (len == 1) >>> + vaddr ^= 3; >>> + else if (len == 2) >>> + vaddr ^= 2; >>> + } >>> +#endif >> >> No target-CPU specific code in exec.c, please... > > Yeah, I'd imagine not. I struggled with this one. Any suggestions for a > better way to do this? You can add a function pointer to CPUClass and call it from here. It's how cc->debug_check_watchpoint is being called already. Paolo