ARM softmmu breakpoint misbehavior

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.

                 } 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]

On 2 September 2015 at 20:08, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> 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.

I think it won't affect architectures that don't set BP_STOP_BEFORE_ACCESS,
because for those we will execute the load before trying to see if there
is a watchpoint to take, and if there's no CPU watchpoint we'll be able
to continue normally. So ARM, LM32, S390x and Xtensa might be affected.
And of course any architecture which only sets up wps/bps which will
definitely be hit architecturally when QEMU thinks they've been hit
will be fine.

Maybe rather than trying to be clever with the existing wp APIs
we should have support for targets to register "did this really hit?"
callbacks that get called before the core code tries to really
generate the exception.

I don't think I can really look in detail at this problem til next
week at the earliest, though.

thanks
-- PMM

On 02.09.2015 22:45, Peter Maydell wrote:
> Maybe rather than trying to be clever with the existing wp APIs
> we should have support for targets to register "did this really hit?"
> callbacks that get called before the core code tries to really
> generate the exception.

Thank you, Peter, for the sensible suggestion. I'm going to stick to
this approach. As of breakpoints, it seems to be enough to use a helper
for that purpose.

> I don't think I can really look in detail at this problem til next
> week at the earliest, though.

I hope I will prepare and send relevant patches by that time :)

Best,
Sergey