qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Sergey Fedorov <serge.fdrv@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] ARM softmmu breakpoint misbehavior
Date: Wed, 2 Sep 2015 22:08:42 +0300	[thread overview]
Message-ID: <55E7493A.7030704@gmail.com> (raw)
In-Reply-To: <55E72981.3050808@gmail.com>

On 02.09.2015 19:53, Sergey Fedorov wrote:
> On 28.08.2015 22:21, Peter Maydell wrote:
>> The watchpoint code has a chance of cpu_resume_from_signal
>> doing the right thing, because we really did have the
>> code to do the load/store. However I have a feeling this
>> won't interact properly with the fact that ARM needs
>> BP_STOP_BEFORE_ACCESS on its watchpoints (unlike x86, which
>> is where I was looking at when I wrote the ARM wp handling
>> code.) So we may well be broken there as well in the
>> case where check_watchpoints() returns false.
> You are right. The same problem with watchpoints. Here is a small test
> for this:
>
>     .text
>     .global _start
> _start:
>     adr     x0, wp
>     msr     dbgwvr0_el1, x0
>     mov     x0, #1
>     orr     x0, x0, #(3 << 3)
>     orr     x0, x0, #(0xff << 5)
>     msr     dbgwcr0_el1, x0
>     ldr     x0, wp
>     wfi
>     b       .
>
>     .data
>     .balign 64
> wp:
>     .quad   0
>

With the following patch the test is okay, but I am not sure that this
is a clean solution. Anyway, we can't do such a simple hack for
breakpoints. Seems that this is a systematic problem which can affect
all architectures.

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 66edbe9..013ac7e 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -878,11 +878,12 @@ void arm_debug_excp_handler(CPUState *cs)
 
     if (wp_hit) {
         if (wp_hit->flags & BP_CPU) {
-            cs->watchpoint_hit = NULL;
             if (check_watchpoints(cpu)) {
                 bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0;
                 bool same_el = arm_debug_target_el(env) ==
arm_current_el(env);
 
+                cs->watchpoint_hit = NULL;
+
                 if (extended_addresses_enabled(env)) {
                     env->exception.fsr = (1 << 9) | 0x22;
                 } else {

I haven't investigated it in detail but the load instruction at
0x40000018 is translated 3 times and there are 2 attempts to execute it.

----------------
IN:
0x0000000040000000:  10080200      adr x0, #+0x10040 (addr 0x40010040)

Trace 0x7fc6aaf09000 [0000000040000000]
----------------
IN:
0x0000000040000004:  d51000c0      msr (unknown), x0

Trace 0x7fc6aaf09040 [0000000040000004]
----------------
IN:
0x0000000040000008:  d2800020      mov x0, #0x1

Trace 0x7fc6aaf09080 [0000000040000008]
----------------
IN:
0x000000004000000c:  b27d0400      orr x0, x0, #0x18

Trace 0x7fc6aaf090c0 [000000004000000c]
----------------
IN:
0x0000000040000010:  b27b1c00      orr x0, x0, #0x1fe0

Trace 0x7fc6aaf09110 [0000000040000010]
----------------
IN:
0x0000000040000014:  d51000e0      msr (unknown), x0

Trace 0x7fc6aaf09160 [0000000040000014]
----------------
IN:
0x0000000040000018:  58080140      ldr x0, pc+65576 (addr 0x40010040)

Trace 0x7fc6aaf091a0 [0000000040000018]
----------------
IN:
0x0000000040000018:  58080140      ldr x0, pc+65576 (addr 0x40010040)

----------------
IN:
0x0000000040000018:  58080140      ldr x0, pc+65576 (addr 0x40010040)

Trace 0x7fc6aaf09230 [0000000040000018]
----------------
IN:
0x000000004000001c:  d503207f      unimplemented (System)

Trace 0x7fc6aaf092c0 [000000004000001c]
Trace 0x7fc6aaf092c0 [000000004000001c]

  reply	other threads:[~2015-09-02 19:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-24 17:36 [Qemu-devel] ARM softmmu breakpoint misbehavior Sergey Fedorov
2015-08-25 20:12 ` Christopher Covington
2015-08-28 19:21 ` Peter Maydell
2015-09-01 11:58   ` Sergey Fedorov
2015-09-02 16:53   ` Sergey Fedorov
2015-09-02 19:08     ` Sergey Fedorov [this message]
2015-09-02 19:45       ` Peter Maydell
2015-09-03  8:39         ` Sergey Fedorov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55E7493A.7030704@gmail.com \
    --to=serge.fdrv@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).