This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] aarch64-core.xml: 32-bit cpsr -> 64-bit pstate
- From: Richard Earnshaw <Richard dot Earnshaw at foss dot arm dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Cc: pinskia at gmail dot com, catalin dot udma at freescale dot com
- Date: Mon, 7 Sep 2015 23:46:27 +0100
- Subject: Re: [PATCH] aarch64-core.xml: 32-bit cpsr -> 64-bit pstate
- Authentication-results: sourceware.org; auth=none
- References: <1441284969-30465-1-git-send-email-yao dot qi at linaro dot org>
On 03/09/15 13:56, Yao Qi wrote:
> This patch is to change 32-bit cpsr to 64-bit pstate in aarch64 target
> description.
>
> When aarch64 GDB port was added into GDB, cpsr is 32-bit in
> aarch64-core.xml. Andrew Pinski changed it to 64-bit
> https://sourceware.org/ml/gdb-patches/2013-12/msg00720.html but forget
> updating GDBserver side. As a result, GDB thinks cpsr is 64-bit, but
> GDBserver still thinks it is 32-bit, so contents of registers after
> cpster, such as floating point registers, are wrong. Catalin Udma
> gave a patch https://sourceware.org/ml/gdb-patches/2014-08/msg00193.html
> to change cpsr in GDBserver side to 64-bit. After some discussions,
> we decided to revert Andrew's patch, and let cpsr be 32-bit.
>
> A problem "Remote 'g' packet reply is too long:" (when I connect gdb
> to kgdb with aarch64 kernel) leads me to the 32-bit vs 64-bit issue
> again.
>
> cpsr is 32-bit, but it only exists aarch32 execution state. As in
> ARMv8 reference manual, "A1.3.1 Execution state"
>
> "The A32 and T32
> instruction sets include instructions that operate directly on various PSTATE elements, and
> instructions that access PSTATE by using the Application Program Status Register (APSR)
> or the Current Program Status Register (CPSR)".
>
> In other words, there is no cpsr at all on aarch64 execution state.
> On aarch64 execution state, pstate is an abstraction of process state
> information, so it has no architecturally defined size or enconding.
>
> However, the architecture does define how the PSTATE is encoded in the
> SPSR when taking an exception, and that is the form of PSTATE in
> ptrace. Currently only bits 31:0 of SPSR are defined, but it is only
> accessible via MSR/MRS instructions, which always transfer 64-bits,
> so on ISA level, it should be regarded as 64-bit.
>
> This change will break the compatibility for <new gdb, old gdbserver>
> and <old gdb, new gdbserver>, but current GDB is wrong, we have to fix it.
>
Won't this also break any situation where the gdbserver knowledge is
embedded in a HW stub? You're assuming that we can just update all
those servers and everything will be fine and dandy. I don't think we
can't just change this arbitrarily.
I think kgdb should do what everyone else has been doing and simply
return the bottom 32 bits.
R.
> gdb:
>
> 2015-09-03 Yao Qi <yao.qi@linaro.org>
>
> * aarch64-tdep.c (aarch64_r_register_names): Rename cpsr to
> pstate.
> * features/aarch64-core.xmli (cpsr): Rename it to pstate, and
> change its size to 64-bit.
> * features/aarch64.c: Regenerate.
> * regformats/aarch64.dat: Likewise.
> ---
> gdb/aarch64-tdep.c | 2 +-
> gdb/features/aarch64-core.xml | 2 +-
> gdb/features/aarch64.c | 2 +-
> gdb/regformats/aarch64.dat | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 5d8ffd4..04064cb 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -127,7 +127,7 @@ static const char *const aarch64_r_register_names[] =
> "x20", "x21", "x22", "x23",
> "x24", "x25", "x26", "x27",
> "x28", "x29", "x30", "sp",
> - "pc", "cpsr"
> + "pc", "pstate"
> };
>
> /* The FP/SIMD 'V' registers. */
> diff --git a/gdb/features/aarch64-core.xml b/gdb/features/aarch64-core.xml
> index c95049c..f0f455f 100644
> --- a/gdb/features/aarch64-core.xml
> +++ b/gdb/features/aarch64-core.xml
> @@ -42,5 +42,5 @@
> <reg name="sp" bitsize="64" type="data_ptr"/>
>
> <reg name="pc" bitsize="64" type="code_ptr"/>
> - <reg name="cpsr" bitsize="32"/>
> + <reg name="pstate" bitsize="64"/>
> </feature>
> diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c
> index 1e9a99d..643387d 100644
> --- a/gdb/features/aarch64.c
> +++ b/gdb/features/aarch64.c
> @@ -50,7 +50,7 @@ initialize_tdesc_aarch64 (void)
> tdesc_create_reg (feature, "x30", 30, 1, NULL, 64, "int");
> tdesc_create_reg (feature, "sp", 31, 1, NULL, 64, "data_ptr");
> tdesc_create_reg (feature, "pc", 32, 1, NULL, 64, "code_ptr");
> - tdesc_create_reg (feature, "cpsr", 33, 1, NULL, 32, "int");
> + tdesc_create_reg (feature, "pstate", 33, 1, NULL, 64, "int");
>
> feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu");
> field_type = tdesc_named_type (feature, "ieee_double");
> diff --git a/gdb/regformats/aarch64.dat b/gdb/regformats/aarch64.dat
> index d4cea04..0db2f3a 100644
> --- a/gdb/regformats/aarch64.dat
> +++ b/gdb/regformats/aarch64.dat
> @@ -36,7 +36,7 @@ expedite:x29,sp,pc
> 64:x30
> 64:sp
> 64:pc
> -32:cpsr
> +64:pstate
> 128:v0
> 128:v1
> 128:v2
>