* [PATCH] AARCH64: Change cpsr type to be 64bit.
@ 2013-12-18 19:08 Andrew Pinski
2013-12-20 17:48 ` Pedro Alves
2013-12-23 17:13 ` Yufeng Zhang
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Pinski @ 2013-12-18 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Pinski
As mentioned in http://www.spinics.net/lists/arm-kernel/msg290896.html, we should change gdb's notion of cpsr to be 64bit.
OK? Build and tested for aarch64-linux-gnu with no regressions.
I also double checked to make sure that what is passed down from the kernel (via sigcontext), is the full 64bit.
Thanks,
Andrew Pinski
2013-12-18 Andrew Pinski <apinski@cavium.com>
* features/aarch64-core.xml (cpsr): Change to be 64bit.
* features/aarch64.c: Regenerate.
---
gdb/ChangeLog | 5 +++++
gdb/features/aarch64-core.xml | 2 +-
gdb/features/aarch64.c | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 513c593..6bde494 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2013-12-18 Andrew Pinski <apinski@cavium.com>
+
+ * features/aarch64-core.xml (cpsr): Change to be 64bit.
+ * features/aarch64.c: Regenerate.
+
2013-12-18 Yufeng Zhang <yufeng.zhang@arm.com>
* aarch64-linux-nat.c (aarch64_linux_set_debug_regs): Set
diff --git a/gdb/features/aarch64-core.xml b/gdb/features/aarch64-core.xml
index 53c63b2..8549eeb 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="cpsr" bitsize="64"/>
</feature>
diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c
index 1e9a99d..31a148e 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, "cpsr", 33, 1, NULL, 64, "int");
feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu");
field_type = tdesc_named_type (feature, "ieee_double");
--
1.7.2.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AARCH64: Change cpsr type to be 64bit.
2013-12-18 19:08 [PATCH] AARCH64: Change cpsr type to be 64bit Andrew Pinski
@ 2013-12-20 17:48 ` Pedro Alves
2013-12-23 17:39 ` Yufeng Zhang
2013-12-23 17:13 ` Yufeng Zhang
1 sibling, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2013-12-20 17:48 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gdb-patches, Andrew Pinski
On 12/18/2013 07:08 PM, Andrew Pinski wrote:
> As mentioned in http://www.spinics.net/lists/arm-kernel/msg290896.html, we should change gdb's notion of cpsr to be 64bit.
>
> OK? Build and tested for aarch64-linux-gnu with no regressions.
> I also double checked to make sure that what is passed down from the kernel (via sigcontext), is the full 64bit.
With this, the register will be exposed as 64-bit to the
user. Is that desirable? The fact that the original
description used 32-bit makes it sounds like it's not.
What's the real register width at the (asm visible)
machine level?
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AARCH64: Change cpsr type to be 64bit.
2013-12-20 17:48 ` Pedro Alves
@ 2013-12-23 17:39 ` Yufeng Zhang
2013-12-23 18:05 ` Mark Kettenis
0 siblings, 1 reply; 6+ messages in thread
From: Yufeng Zhang @ 2013-12-23 17:39 UTC (permalink / raw)
To: Pedro Alves; +Cc: Andrew Pinski, gdb-patches, Andrew Pinski
On 12/20/13 17:47, Pedro Alves wrote:
> On 12/18/2013 07:08 PM, Andrew Pinski wrote:
>> As mentioned in http://www.spinics.net/lists/arm-kernel/msg290896.html, we should change gdb's notion of cpsr to be 64bit.
>>
>> OK? Build and tested for aarch64-linux-gnu with no regressions.
>> I also double checked to make sure that what is passed down from the kernel (via sigcontext), is the full 64bit.
>
> With this, the register will be exposed as 64-bit to the
> user. Is that desirable? The fact that the original
> description used 32-bit makes it sounds like it's not.
> What's the real register width at the (asm visible)
> machine level?
There is no access to CPSR as a single register in AArch64 (*).
Instead, process states can be accessed/modified via system
instructions. I guess the kernel synthesizes one CPSR register, so if
kernel defines it as a 64-bit register, it is reasonable for gdb to
treat it of the same size as well.
Yufeng
Reference:
* ARMv8 Instruction Set Overview - Chapter 3 A64 Overview
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AARCH64: Change cpsr type to be 64bit.
2013-12-23 17:39 ` Yufeng Zhang
@ 2013-12-23 18:05 ` Mark Kettenis
2013-12-24 10:23 ` Yufeng Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2013-12-23 18:05 UTC (permalink / raw)
To: Yufeng.Zhang; +Cc: palves, pinskia, gdb-patches, apinski
> Date: Mon, 23 Dec 2013 17:39:00 +0000
> From: Yufeng Zhang <Yufeng.Zhang@arm.com>
>
> On 12/20/13 17:47, Pedro Alves wrote:
> > On 12/18/2013 07:08 PM, Andrew Pinski wrote:
> >> As mentioned in http://www.spinics.net/lists/arm-kernel/msg290896.html, we should change gdb's notion of cpsr to be 64bit.
> >>
> >> OK? Build and tested for aarch64-linux-gnu with no regressions.
> >> I also double checked to make sure that what is passed down from the kernel (via sigcontext), is the full 64bit.
> >
> > With this, the register will be exposed as 64-bit to the
> > user. Is that desirable? The fact that the original
> > description used 32-bit makes it sounds like it's not.
> > What's the real register width at the (asm visible)
> > machine level?
>
> There is no access to CPSR as a single register in AArch64 (*).
> Instead, process states can be accessed/modified via system
> instructions. I guess the kernel synthesizes one CPSR register, so if
> kernel defines it as a 64-bit register, it is reasonable for gdb to
> treat it of the same size as well.
Basing GDB's fundamentals on a particular OS's ptrace(2)
implementation is a bad idea.
> Reference:
>
> * ARMv8 Instruction Set Overview - Chapter 3 A64 Overview
Wow, is the ARMv8 instruction set documentation really only available
to "ARM Custumers"? How are we supposed to review patches for an
architecture for which we don't have access to the fundamental
documentation?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AARCH64: Change cpsr type to be 64bit.
2013-12-23 18:05 ` Mark Kettenis
@ 2013-12-24 10:23 ` Yufeng Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Yufeng Zhang @ 2013-12-24 10:23 UTC (permalink / raw)
To: Mark Kettenis; +Cc: palves, pinskia, gdb-patches, apinski
On 12/23/13 18:04, Mark Kettenis wrote:
>> Date: Mon, 23 Dec 2013 17:39:00 +0000
>> From: Yufeng Zhang<Yufeng.Zhang@arm.com>
>>
>> On 12/20/13 17:47, Pedro Alves wrote:
>>> On 12/18/2013 07:08 PM, Andrew Pinski wrote:
>>>> As mentioned in http://www.spinics.net/lists/arm-kernel/msg290896.html, we should change gdb's notion of cpsr to be 64bit.
>>>>
>>>> OK? Build and tested for aarch64-linux-gnu with no regressions.
>>>> I also double checked to make sure that what is passed down from the kernel (via sigcontext), is the full 64bit.
>>>
>>> With this, the register will be exposed as 64-bit to the
>>> user. Is that desirable? The fact that the original
>>> description used 32-bit makes it sounds like it's not.
>>> What's the real register width at the (asm visible)
>>> machine level?
>>
>> There is no access to CPSR as a single register in AArch64 (*).
>> Instead, process states can be accessed/modified via system
>> instructions. I guess the kernel synthesizes one CPSR register, so if
>> kernel defines it as a 64-bit register, it is reasonable for gdb to
>> treat it of the same size as well.
>
> Basing GDB's fundamentals on a particular OS's ptrace(2)
> implementation is a bad idea.
Do you have a suggestion on how to do it more properly?
>
>> Reference:
>>
>> * ARMv8 Instruction Set Overview - Chapter 3 A64 Overview
>
> Wow, is the ARMv8 instruction set documentation really only available
> to "ARM Custumers"? How are we supposed to review patches for an
> architecture for which we don't have access to the fundamental
> documentation?
>
It is 'customer' in a broader sense. I think the requirement on having
an account is for the purpose of protecting ARM IP. While it is fairly
easy to open an account, I understand that it still causes
inconvenience. I hope the situation will be improved in the future.
Thanks,
Yufeng
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] AARCH64: Change cpsr type to be 64bit.
2013-12-18 19:08 [PATCH] AARCH64: Change cpsr type to be 64bit Andrew Pinski
2013-12-20 17:48 ` Pedro Alves
@ 2013-12-23 17:13 ` Yufeng Zhang
1 sibling, 0 replies; 6+ messages in thread
From: Yufeng Zhang @ 2013-12-23 17:13 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gdb-patches, Andrew Pinski
Looks OK but I cannot approve it.
On 12/18/13 19:08, Andrew Pinski wrote:
> As mentioned in http://www.spinics.net/lists/arm-kernel/msg290896.html, we should change gdb's notion of cpsr to be 64bit.
>
> OK? Build and tested for aarch64-linux-gnu with no regressions.
Assuming that you meant no regression in the big-endian environment, can
you please double check to make sure it works well in the little-endian
environment?
Thanks,
Yufeng
> I also double checked to make sure that what is passed down from the kernel (via sigcontext), is the full 64bit.
>
> Thanks,
> Andrew Pinski
>
> 2013-12-18 Andrew Pinski<apinski@cavium.com>
>
> * features/aarch64-core.xml (cpsr): Change to be 64bit.
> * features/aarch64.c: Regenerate.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/features/aarch64-core.xml | 2 +-
> gdb/features/aarch64.c | 2 +-
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 513c593..6bde494 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2013-12-18 Andrew Pinski<apinski@cavium.com>
> +
> + * features/aarch64-core.xml (cpsr): Change to be 64bit.
> + * features/aarch64.c: Regenerate.
> +
> 2013-12-18 Yufeng Zhang<yufeng.zhang@arm.com>
>
> * aarch64-linux-nat.c (aarch64_linux_set_debug_regs): Set
> diff --git a/gdb/features/aarch64-core.xml b/gdb/features/aarch64-core.xml
> index 53c63b2..8549eeb 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="cpsr" bitsize="64"/>
> </feature>
> diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c
> index 1e9a99d..31a148e 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, "cpsr", 33, 1, NULL, 64, "int");
>
> feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu");
> field_type = tdesc_named_type (feature, "ieee_double");
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-24 10:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-18 19:08 [PATCH] AARCH64: Change cpsr type to be 64bit Andrew Pinski
2013-12-20 17:48 ` Pedro Alves
2013-12-23 17:39 ` Yufeng Zhang
2013-12-23 18:05 ` Mark Kettenis
2013-12-24 10:23 ` Yufeng Zhang
2013-12-23 17:13 ` Yufeng Zhang
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 read-only IMAP folder(s) and NNTP newsgroup(s).