lkml.org 
[lkml]   [2016]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] arm64: kgdb: Match pstate size with gdbserver protocol
From
Date
On 10/05/16 15:31, Mark Rutland wrote:
> On Tue, May 10, 2016 at 02:41:54PM +0100, Daniel Thompson wrote:
>> On 10/05/16 11:45, Mark Rutland wrote:
>>>> When commit 44679a4f142b ("arm64: KGDB: Add step debugging support") was
>>>> introduced it was paired with a gdb patch that made an incompatible
>>>> change to the gdbserver protocol. This patch was eventually merged into
>>>> the gdb sources:
>>>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=a4d9ba85ec5597a6a556afe26b712e878374b9dd
>>>>
>>>> The change to the protocol was mostly made to simplify big-endian support
>>>> inside the kernel gdb stub.
>>>
>>> While that was how we discovered the inconsistency, a major concern is
>>> that SPSR_EL* (i.e. PSTATE), as accessed by MRS/MSR is a 64-bit
>>> quantity, even if the upper 32 bits are RES0 today.
>>>
>>> It is conceivable that the upper 32 bits could be used in future (as
>>> happened with CLIDR_EL1), and for this reason we expose those upper 32
>>> bits from the kernel, and treat system registers as 64-bit quantities
>>> generally.
>>
>> These cases are not exactly the same.
>>
>> CLIDR_ELx is (or was) architecturally defined as a 64-bit register
>> and explicitly marks the upper 32-bits as RES0.
>>
>> That is not the case for SPSR_ELx; this register is architecturally
>> defined to be 32-bit.
>
> The below doesn't necessarily change your subsequent argument, but that
> isn't quite true.
>
> The two cases are in fact identical if you dig into the history a bit
> further. Take a look in an earlier revision of the ARM ARM (e.g. ARM DDI
> 0487A.b), where it was explicitly stated:
>
> Attributes
> CLIDR_EL1 is a 32-bit register.

I see. I only when back as far as A.e ...


> Based on this, my PoV is that any register that the ARM ARM describes as
> "a 32-bit register" is a 64-bit register for which the upper 32 bits are
> RES0.

Understood.

>> I know that doesn't *prevent* SPSR_ELx from being expanded in the
>> future it is not unreasonable for gdb to design its wire protocol
>> based on the description found in the architecture manual.
>>
>>> So this was also about ensuring the interface was consistent and to some
>>> extent future-proof.
>>
>> gdb remote protocol is already future proof and has never at any
>> point contradicted the architecture.
>>
>> However the changes to the protocol in 7.7.x and 7.8.0 were
>> analogous to an unexpected ABI change rather than a carefully
>> controlled introduction of a new feature. Like the kernel, once
>> detected, they were reversed ;-) .
>
> To be clear, I don't disagree with this. The ARM ARM is at best
> amigibious w.r.t. what it means by "a 32-bit register", and that's the
> only point of contention.

Agree... such ambiguity strikes me as a particularly serious problem for
switchable context registers since it takes *very* close attention to
the ARM ARM for an existing OS to determine that it is required to treat
the upper 32-bits of context should-be-zero-or-preserved.


> However, we should make note of the above as a key point of rationale,
> as it affects other decisions we make in this area.

Ok. I'll add comments describing how the gdbremote protocol is
interpreting the spec.


Daniel.

\
 
 \ /
  Last update: 2016-05-12 15:21    [W:0.053 / U:0.964 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site