Re: [RFA/commit] arm-tdep.c: Do not single-step after hitting a watchpoint

[Pedro Alves <palves at redhat dot com>]

On 09/30/2014 11:00 AM, Peter Maydell wrote:
> On 30 September 2014 10:08, Pedro Alves <palves@redhat.com> wrote:
>> On 09/29/2014 11:53 PM, Peter Maydell wrote:
>>> There's an assertion in this LKML post from 2010:
>>> https://lkml.org/lkml/2010/4/14/127
>>> that v7 cores do actually all generate synchronous
>>> watchpoint exceptions (even though architecturally
>>> they're permitted not to). Was your test h/w a v6?
>>
>> Joel's test was against qemu (without your patch).
>>
>> Terry's tests were against armv7l and armv8.  Both synchronous.
>>
>> The report that confuses me is Gareth's:
>>
>>   https://sourceware.org/ml/gdb/2014-09/msg00013.html
>>
>> As it sounds like he has v7-m hardware that has asynchronous
>> behavior.  Gareth, can you confirm this, please?
> 
> In general it's unwise to assume that statements
> about the ARM A and R profiles carry across to M
> profile... 

Ah.  Noted.

> v7M profile watchpoints are rather
> different from v7AR watchpoints in terms of how you
> set them, how they're reported, etc, and they're
> always asynchronous (other insns may execute after
> the one which triggers the wp before the debug event
> fires).
> 
>> Still, in any case, from that LKML post:
>>
>>  "v6 cores are the opposite; they only generate asynchronous
>>   watchpoint exceptions".
>>
>> So, eh!?  Does your qemu patch take this into account?  Seems
>> like it should.
> 
> My QEMU patch is for the built in gdbstub, which is
> completely different code to the emulation of the
> CPU's own architected debug hardware. (We implement
> the latter only for v7 and above, not v6.)
> 
> It doesn't seem very sensible to me to deliberately
> provide unhelpful asynchronous watchpoint support
> on v6-and-lower guest CPUs just because that's what
> the hardware does, especially since it would mean we
> wouldn't interoperate with current gdb. 

But current GDB is wrong, and it wasn't always wrong.

I see now that GDB switched to assuming synchronous
watchpoints not that long ago:

  https://sourceware.org/ml/gdb-patches/2010-07/msg00110.html

That's git e3039479.

That was only when we added support for Linux watchpoints, after
7.2.  It seems like non-Linux / bare-metal was forgotten in
that patch, and so that broke qemu watchpoints.

IOW, in GDB 7.2 and before, GDB assumed asynchronous watchpoints
on ARM, and that's what qemu implemented.

So that was really a regression that went by unnoticed.

Probably nobody complained so far because usually one
doesn't notice GDB stopped execution one instruction too
far.  See Joel's hypothesis, which I agree with:

  https://sourceware.org/ml/gdb/2014-09/msg00015.html

> (Similarly,
> we provide watchpoint support in our stub even if
> the CPU we're emulating has no watchpoint support
> of its own at all. Think of us as like a JTAG probe.)

Well, it seems to me that GDB, on v6-and-lower is
doing the wrong thing for real halt-mode/jtag probes.
If we fix that in GDB, then your qemu patch breaks
things on v6.

> 
>> Now I'm confused on the mention of the Linux kernel
>> subtracting 8 from the PC to help GDB.  I can't find that
>> anywhere in the kernel's sources.
> 
> This is a reference to the standard ARM exception
> entry behaviour where the value saved to the link
> register may be +2, +4 or +8 from the "preferred
> return address" for the exception. The kernel handles
> this via a 'vector_stub' macro that adjusts the
> value read from LR so the rest of the kernel can
> deal simply in preferred return addresses. Since
> sync. watchpoints are a kind of data abort they
> go through here, with a correction value of 8:
> http://lxr.free-electrons.com/source/arch/arm/kernel/entry-armv.S#L1043

Ah.  Thanks.

Thanks,
Pedro Alves