public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [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-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

* 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

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).