public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] Add support for ARMv7M devices.
@ 2010-06-09 14:03 Kazu Hirata
  2010-06-09 22:29 ` Doug Evans
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Kazu Hirata @ 2010-06-09 14:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: dan

Hi,

Attached is a patch to add support for ARMv7M devices.

The patch is bigger than it really is due to the signature change to
arm_pc_is_thumb.

Here are some highlights:

- arm_pc_is_thumb always returns 1 if the target is an ARMv7M device.

- arm_write_pc is adjusted so that it will set the Thumb bit in XPSR
  for an ARMv7M device.  Note that the position of Thumb bit in the
  status register is different between ARMv7M devices and older
  devices.

- arm_gdbarch_init looks for "org.gnu.gdb.arm.core-v7m".

Tested by running Hello, world on a Cortex-M3 device.  OK to apply?

Kazu Hirata

2010-06-08  Daniel Jacobowitz  <dan@codesourcery.com>
	    Kazu Hirata  <kazu@codesourcery.com>

	* arm-tdep.c (arm_pc_is_thumb): Return 1 if the target is an
	M-profile device.
	(arm_write_pc): Handle an M-profile device.
	(arm_gdbarch_init): Handle an M-profile device.
	* arm-tdep.h (XPSR_T): New.
	(gdbarch_tdep): Add is_m.

Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.304
diff -u -d -p -r1.304 arm-tdep.c
--- gdb/arm-tdep.c	27 May 2010 19:06:12 -0000	1.304
+++ gdb/arm-tdep.c	9 Jun 2010 03:12:16 -0000
@@ -347,12 +347,16 @@ static CORE_ADDR arm_get_next_pc_raw (st
    any executing frame; otherwise, prefer arm_frame_is_thumb.  */
 
 static int
-arm_pc_is_thumb (CORE_ADDR memaddr)
+arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr)
 {
   struct obj_section *sec;
   struct minimal_symbol *sym;
   char type;
 
+  /* ARMv7M is always in Thumb mode.  */
+  if (gdbarch_tdep (gdbarch)->is_m)
+    return 1;
+
   /* If bit 0 of the address is set, assume this is a Thumb address.  */
   if (IS_THUMB_ADDR (memaddr))
     return 1;
@@ -815,7 +819,7 @@ arm_skip_prologue (struct gdbarch *gdbar
 	     associate prologue code with the opening brace; so this
 	     lets us skip the first line if we think it is the opening
 	     brace.  */
-	  if (arm_pc_is_thumb (func_addr))
+	  if (arm_pc_is_thumb (gdbarch, func_addr))
 	    analyzed_limit = thumb_analyze_prologue (gdbarch, func_addr,
 						     post_prologue_pc, NULL);
 	  else
@@ -842,7 +846,7 @@ arm_skip_prologue (struct gdbarch *gdbar
 
 
   /* Check if this is Thumb code.  */
-  if (arm_pc_is_thumb (pc))
+  if (arm_pc_is_thumb (gdbarch, pc))
     return thumb_analyze_prologue (gdbarch, pc, limit_pc, NULL);
 
   for (skip_pc = pc; skip_pc < limit_pc; skip_pc += 4)
@@ -2008,7 +2012,7 @@ arm_push_dummy_call (struct gdbarch *gdb
 
   /* Set the return address.  For the ARM, the return breakpoint is
      always at BP_ADDR.  */
-  if (arm_pc_is_thumb (bp_addr))
+  if (arm_pc_is_thumb (gdbarch, bp_addr))
     bp_addr |= 1;
   regcache_cooked_write_unsigned (regcache, ARM_LR_REGNUM, bp_addr);
 
@@ -2147,7 +2151,7 @@ arm_push_dummy_call (struct gdbarch *gdb
 	  && TYPE_CODE_FUNC == TYPE_CODE (target_type))
 	{
 	  CORE_ADDR regval = extract_unsigned_integer (val, len, byte_order);
-	  if (arm_pc_is_thumb (regval))
+	  if (arm_pc_is_thumb (gdbarch, regval))
 	    {
 	      bfd_byte *copy = alloca (len);
 	      store_unsigned_integer (copy, len, byte_order,
@@ -3352,7 +3356,7 @@ arm_adjust_breakpoint_address (struct gd
     return bpaddr;
 
   /* ARM mode does not have this problem.  */
-  if (!arm_pc_is_thumb (bpaddr))
+  if (!arm_pc_is_thumb (gdbarch, bpaddr))
     return bpaddr;
 
   /* We are setting a breakpoint in Thumb code that could potentially
@@ -5345,7 +5349,9 @@ arm_displaced_step_fixup (struct gdbarch
 static int
 gdb_print_insn_arm (bfd_vma memaddr, disassemble_info *info)
 {
-  if (arm_pc_is_thumb (memaddr))
+  struct gdbarch *gdbarch = info->application_data;
+
+  if (arm_pc_is_thumb (gdbarch, memaddr))
     {
       static asymbol *asym;
       static combined_entry_type ce;
@@ -5435,7 +5441,7 @@ arm_breakpoint_from_pc (struct gdbarch *
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
 
-  if (arm_pc_is_thumb (*pcptr))
+  if (arm_pc_is_thumb (gdbarch, *pcptr))
     {
       *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
 
@@ -5474,7 +5480,7 @@ arm_remote_breakpoint_from_pc (struct gd
 
   arm_breakpoint_from_pc (gdbarch, pcptr, kindptr);
 
-  if (arm_pc_is_thumb (*pcptr) && *kindptr == 4)
+  if (arm_pc_is_thumb (gdbarch, *pcptr) && *kindptr == 4)
     /* The documented magic value for a 32-bit Thumb-2 breakpoint, so
        that this is not confused with a 32-bit ARM breakpoint.  */
     *kindptr = 3;
@@ -6219,6 +6225,7 @@ arm_record_special_symbol (struct gdbarc
 static void
 arm_write_pc (struct regcache *regcache, CORE_ADDR pc)
 {
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
   regcache_cooked_write_unsigned (regcache, ARM_PC_REGNUM, pc);
 
   /* If necessary, set the T bit.  */
@@ -6226,11 +6233,18 @@ arm_write_pc (struct regcache *regcache,
     {
       ULONGEST val;
       regcache_cooked_read_unsigned (regcache, ARM_PS_REGNUM, &val);
-      if (arm_pc_is_thumb (pc))
-	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM, val | CPSR_T);
+      if (!gdbarch_tdep (gdbarch)->is_m)
+	{
+	  if (arm_pc_is_thumb (gdbarch, pc))
+	    regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
+					    val | CPSR_T);
+	  else
+	    regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
+					    val & ~(ULONGEST) CPSR_T);
+	}
       else
 	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
-					val & ~(ULONGEST) CPSR_T);
+					val | XPSR_T);
     }
 }
 
@@ -6411,7 +6425,7 @@ arm_gdbarch_init (struct gdbarch_info in
   enum arm_abi_kind arm_abi = arm_abi_global;
   enum arm_float_model fp_model = arm_fp_model;
   struct tdesc_arch_data *tdesc_data = NULL;
-  int i;
+  int i, is_m = 0;
   int have_vfp_registers = 0, have_vfp_pseudos = 0, have_neon_pseudos = 0;
   int have_neon = 0;
   int have_fpa_registers = 1;
@@ -6431,7 +6445,14 @@ arm_gdbarch_init (struct gdbarch_info in
       feature = tdesc_find_feature (info.target_desc,
 				    "org.gnu.gdb.arm.core");
       if (feature == NULL)
-	return NULL;
+	{
+	  feature = tdesc_find_feature (info.target_desc,
+					"org.gnu.gdb.arm.core-v7m");
+	  if (feature == NULL)
+	    return NULL;
+	  else
+	    is_m = 1;
+	}
 
       tdesc_data = tdesc_data_alloc ();
 
@@ -6448,8 +6469,12 @@ arm_gdbarch_init (struct gdbarch_info in
       valid_p &= tdesc_numbered_register_choices (feature, tdesc_data,
 						  ARM_PC_REGNUM,
 						  arm_pc_names);
-      valid_p &= tdesc_numbered_register (feature, tdesc_data,
-					  ARM_PS_REGNUM, "cpsr");
+      if (is_m)
+	valid_p &= tdesc_numbered_register (feature, tdesc_data,
+					    ARM_PS_REGNUM, "xPSR");
+      else
+	valid_p &= tdesc_numbered_register (feature, tdesc_data,
+					    ARM_PS_REGNUM, "cpsr");
 
       if (!valid_p)
 	{
@@ -6735,6 +6760,7 @@ arm_gdbarch_init (struct gdbarch_info in
      These are gdbarch discriminators, like the OSABI.  */
   tdep->arm_abi = arm_abi;
   tdep->fp_model = fp_model;
+  tdep->is_m = is_m;
   tdep->have_fpa_registers = have_fpa_registers;
   tdep->have_vfp_registers = have_vfp_registers;
   tdep->have_vfp_pseudos = have_vfp_pseudos;
Index: gdb/arm-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.h,v
retrieving revision 1.40
diff -u -d -p -r1.40 arm-tdep.h
--- gdb/arm-tdep.h	12 Apr 2010 13:52:43 -0000	1.40
+++ gdb/arm-tdep.h	9 Jun 2010 03:12:16 -0000
@@ -108,6 +108,8 @@ enum gdb_regnum {
 
 #define CPSR_T		0x20
 
+#define XPSR_T		0x01000000
+
 /* Type of floating-point code in use by inferior.  There are really 3 models
    that are traditionally supported (plus the endianness issue), but gcc can
    only generate 2 of those.  The third is APCS_FLOAT, where arguments to
@@ -163,6 +165,7 @@ struct gdbarch_tdep
 				   have_vfp_pseudos.  */
   int have_neon;		/* Do we have a NEON unit?  */
 
+  int is_m;			/* Does the target follow the "M" profile.  */
   CORE_ADDR lowest_pc;		/* Lowest address at which instructions 
 				   will appear.  */
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-06-09 14:03 [patch] Add support for ARMv7M devices Kazu Hirata
@ 2010-06-09 22:29 ` Doug Evans
  2010-06-09 23:04 ` Richard Earnshaw
  2010-06-10  8:56 ` Matthew Gretton-Dann
  2 siblings, 0 replies; 14+ messages in thread
From: Doug Evans @ 2010-06-09 22:29 UTC (permalink / raw)
  To: Kazu Hirata; +Cc: gdb-patches, dan

On Wed, Jun 9, 2010 at 7:03 AM, Kazu Hirata <kazu@codesourcery.com> wrote:
> Hi,
>
> Attached is a patch to add support for ARMv7M devices.
>
> The patch is bigger than it really is due to the signature change to
> arm_pc_is_thumb.
>
> Here are some highlights:
>
> - arm_pc_is_thumb always returns 1 if the target is an ARMv7M device.
> [...]

No objections for my part, but a nit, and not one that you necessarily
have to address.
Options "arm fallback-mode" and "arm force-mode" are confusing, at
least if you read their help string without digging into the
implementation.
IWBN if the help text specified that they do not apply if the mode can
be discerned from the h/w, and list the ways in which that is done.
In some sense it could be thought of as being obvious (why want
something different from what the h/w says?).
Maybe I'm being excessively pedantic on the use of the word "force".

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-06-09 14:03 [patch] Add support for ARMv7M devices Kazu Hirata
  2010-06-09 22:29 ` Doug Evans
@ 2010-06-09 23:04 ` Richard Earnshaw
  2010-06-24 16:42   ` Daniel Jacobowitz
  2010-06-10  8:56 ` Matthew Gretton-Dann
  2 siblings, 1 reply; 14+ messages in thread
From: Richard Earnshaw @ 2010-06-09 23:04 UTC (permalink / raw)
  To: Kazu Hirata; +Cc: gdb-patches, dan

On 09/06/10 15:03, Kazu Hirata wrote:
> Hi,
>
> Attached is a patch to add support for ARMv7M devices.
>
> The patch is bigger than it really is due to the signature change to
> arm_pc_is_thumb.
>
> Here are some highlights:
>
> - arm_pc_is_thumb always returns 1 if the target is an ARMv7M device.
>
> - arm_write_pc is adjusted so that it will set the Thumb bit in XPSR
>    for an ARMv7M device.  Note that the position of Thumb bit in the
>    status register is different between ARMv7M devices and older
>    devices.
>
> - arm_gdbarch_init looks for "org.gnu.gdb.arm.core-v7m".
>

It looks to me as though this code should also mostly apply to ARMv6M as 
well (coretex-m0/1).  However, the choice of name 
'org.gnu.gdb.arm.core-v7m' seems to suggest otherwise.  Is there really 
a difference between the support needed for these cores?  Or would a 
more suitable name be appropriate?

R.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-06-09 14:03 [patch] Add support for ARMv7M devices Kazu Hirata
  2010-06-09 22:29 ` Doug Evans
  2010-06-09 23:04 ` Richard Earnshaw
@ 2010-06-10  8:56 ` Matthew Gretton-Dann
  2 siblings, 0 replies; 14+ messages in thread
From: Matthew Gretton-Dann @ 2010-06-10  8:56 UTC (permalink / raw)
  To: Kazu Hirata; +Cc: gdb-patches, dan

Hi,

On Wed, 2010-06-09 at 07:03 -0700, Kazu Hirata wrote:
> Hi,
> 
> Attached is a patch to add support for ARMv7M devices.
> 
> The patch is bigger than it really is due to the signature change to
> arm_pc_is_thumb.
> 
> Index: gdb/arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.304
> @@ -6431,7 +6445,14 @@ arm_gdbarch_init (struct gdbarch_info in
>        feature = tdesc_find_feature (info.target_desc,
>  				    "org.gnu.gdb.arm.core");
>        if (feature == NULL)
> -	return NULL;
> +	{
> +	  feature = tdesc_find_feature (info.target_desc,
> +					"org.gnu.gdb.arm.core-v7m");
> +	  if (feature == NULL)
> +	    return NULL;
> +	  else
> +	    is_m = 1;
> +	}
>  
>        tdesc_data = tdesc_data_alloc ();
>  

I don't see a definition of the "org.gnu.gdb.arm.core-v7m" feature
anywhere in the sources.  Can you please add the appropriate XML and
derived C files to the patch - or am I missing something?

Also, I think the core-v7m feature should be an extension/synonym for a
core-v6m feature rather than a new feature in its own right (see Richard
Earnshaw's comments).

Thanks,

Matt


-- 
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-06-09 23:04 ` Richard Earnshaw
@ 2010-06-24 16:42   ` Daniel Jacobowitz
  2010-06-24 20:04     ` Richard Earnshaw
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Jacobowitz @ 2010-06-24 16:42 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: Kazu Hirata, gdb-patches

On Thu, Jun 10, 2010 at 12:04:43AM +0100, Richard Earnshaw wrote:
> It looks to me as though this code should also mostly apply to ARMv6M
> as well (coretex-m0/1).  However, the choice of name
> 'org.gnu.gdb.arm.core-v7m' seems to suggest otherwise.  Is there
> really a difference between the support needed for these cores?  Or
> would a more suitable name be appropriate?

Historical note: it's this way because the patch predated v6m.  We
need to support the v7m name internally for compatibility, but we
can rename the official version if you like.

I can imagine GDB wanting to know about BASEPRI / FAULTMASK.  But we
provide the privileged registers in another "feature" today, and use
this core-v7m feature with both v6-m and v7-m.  So
'org.gnu.gdb.amr.core-m' instead?

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-06-24 16:42   ` Daniel Jacobowitz
@ 2010-06-24 20:04     ` Richard Earnshaw
  2010-08-16 18:06       ` Daniel Jacobowitz
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Earnshaw @ 2010-06-24 20:04 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Kazu Hirata, gdb-patches

On 24/06/10 17:41, Daniel Jacobowitz wrote:
>
> On Thu, Jun 10, 2010 at 12:04:43AM +0100, Richard Earnshaw wrote:
>> It looks to me as though this code should also mostly apply to ARMv6M
>> as well (coretex-m0/1).  However, the choice of name
>> 'org.gnu.gdb.arm.core-v7m' seems to suggest otherwise.  Is there
>> really a difference between the support needed for these cores?  Or
>> would a more suitable name be appropriate?
>
> Historical note: it's this way because the patch predated v6m.  We
> need to support the v7m name internally for compatibility, but we
> can rename the official version if you like.
>
> I can imagine GDB wanting to know about BASEPRI / FAULTMASK.  But we
> provide the privileged registers in another "feature" today, and use
> this core-v7m feature with both v6-m and v7-m.  So
> 'org.gnu.gdb.amr.core-m' instead?
>

I'd suggest

org.gnu.gdb.arm.m-profile




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-06-24 20:04     ` Richard Earnshaw
@ 2010-08-16 18:06       ` Daniel Jacobowitz
  2010-08-17  9:00         ` Matthew Gretton-Dann
  2010-10-29 23:47         ` Jonathan Larmour
  0 siblings, 2 replies; 14+ messages in thread
From: Daniel Jacobowitz @ 2010-08-16 18:06 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: Kazu Hirata, gdb-patches, Jonathan Larmour

On Thu, Jun 24, 2010 at 09:03:52PM +0100, Richard Earnshaw wrote:
> I'd suggest
> 
> org.gnu.gdb.arm.m-profile

Here's a version addressing comments from Richard, Matthew, and Doug,
and also incorporating Jonathan Larmour's changes.

I've moved the is_m check lower in arm_pc_is_thumb, to make "set arm
force-mode" continue to do something sensible; you could arguably push
it further down, but this is at least an improvement.

Tested on ARM EABI and by hand; any comments?  Jonathan, how's this
work for you?  If your target does not give GDB any registers, then
this ought to just switch an M-profile binary over to the xpsr.  If
your debug stub previously sends a bogus "cpsr", then it's hard to say
what will happen.  I wouldn't be surprised at an error - any target
which does that, probably also supplies the FPA registers.  If it's
really necessary, we can try to auto-detect that case in the remote
protocol, but really this is what the XML register descriptions are
for.

-- 
Daniel Jacobowitz
CodeSourcery

2010-08-16  Daniel Jacobowitz  <dan@codesourcery.com>
	    Kazu Hirata  <kazu@codesourcery.com>
	    Jonathan Larmour  <jifl@eCosCentric.com>

	* arm-tdep.c: Include features/arm-with-m.c.
	(arm_psr_thumb_bit): New.  Update all uses of CPSR_T to
	call this function.
	(arm_pc_is_thumb): Add a gdbarch argument.  Update all callers.
	Check is_m after force-mode.
	(arm_gdbarch_init): Check the binary before the target description.
	Add check for M profile attribute.  If we have an M-profile device,
	but no target register description, use arm-with-m.  Recognize the
	new org.gnu.gdb.arm.m-profile feature and its xpsr register.
	(_initialize_arm_tdep): Call initialize_tdesc_arm_with_m.
	* arm-tdep.h (XPSR_T): Define.
	(struct gdbarch_tdep): Add is_m member.
	* features/arm-m-profile.xml, features/arm-with-m.c,
	features/arm-with-m.xml: New files.

	* gdb.texinfo (ARM Features): Document
	org.gnu.gdb.arm.m-profile.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.304
diff -u -p -r1.304 arm-tdep.c
--- arm-tdep.c	27 May 2010 19:06:12 -0000	1.304
+++ arm-tdep.c	16 Aug 2010 16:00:57 -0000
@@ -53,6 +53,8 @@
 #include "gdb_assert.h"
 #include "vec.h"
 
+#include "features/arm-with-m.c"
+
 static int arm_debug;
 
 /* Macros for setting and testing a bit in a minimal symbol that marks
@@ -255,12 +257,24 @@ static CORE_ADDR arm_analyze_prologue (s
 
 int arm_apcs_32 = 1;
 
+/* Return the bit mask in ARM_PS_REGNUM that indicates Thumb mode.  */
+
+static int
+arm_psr_thumb_bit (struct gdbarch *gdbarch)
+{
+  if (gdbarch_tdep (gdbarch)->is_m)
+    return XPSR_T;
+  else
+    return CPSR_T;
+}
+
 /* Determine if FRAME is executing in Thumb mode.  */
 
 static int
 arm_frame_is_thumb (struct frame_info *frame)
 {
   CORE_ADDR cpsr;
+  ULONGEST t_bit = arm_psr_thumb_bit (get_frame_arch (frame));
 
   /* Every ARM frame unwinder can unwind the T bit of the CPSR, either
      directly (from a signal frame or dummy frame) or by interpreting
@@ -268,7 +282,7 @@ arm_frame_is_thumb (struct frame_info *f
      trust the unwinders.  */
   cpsr = get_frame_register_unsigned (frame, ARM_PS_REGNUM);
 
-  return (cpsr & CPSR_T) != 0;
+  return (cpsr & t_bit) != 0;
 }
 
 /* Callback for VEC_lower_bound.  */
@@ -347,7 +361,7 @@ static CORE_ADDR arm_get_next_pc_raw (st
    any executing frame; otherwise, prefer arm_frame_is_thumb.  */
 
 static int
-arm_pc_is_thumb (CORE_ADDR memaddr)
+arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr)
 {
   struct obj_section *sec;
   struct minimal_symbol *sym;
@@ -363,6 +377,10 @@ arm_pc_is_thumb (CORE_ADDR memaddr)
   if (strcmp (arm_force_mode_string, "thumb") == 0)
     return 1;
 
+  /* ARM v6-M and v7-M are always in Thumb mode.  */
+  if (gdbarch_tdep (gdbarch)->is_m)
+    return 1;
+
   /* If there are mapping symbols, consult them.  */
   type = arm_find_mapping_symbol (memaddr, NULL);
   if (type)
@@ -815,7 +833,7 @@ arm_skip_prologue (struct gdbarch *gdbar
 	     associate prologue code with the opening brace; so this
 	     lets us skip the first line if we think it is the opening
 	     brace.  */
-	  if (arm_pc_is_thumb (func_addr))
+	  if (arm_pc_is_thumb (gdbarch, func_addr))
 	    analyzed_limit = thumb_analyze_prologue (gdbarch, func_addr,
 						     post_prologue_pc, NULL);
 	  else
@@ -842,7 +860,7 @@ arm_skip_prologue (struct gdbarch *gdbar
 
 
   /* Check if this is Thumb code.  */
-  if (arm_pc_is_thumb (pc))
+  if (arm_pc_is_thumb (gdbarch, pc))
     return thumb_analyze_prologue (gdbarch, pc, limit_pc, NULL);
 
   for (skip_pc = pc; skip_pc < limit_pc; skip_pc += 4)
@@ -1507,13 +1525,14 @@ arm_prologue_prev_register (struct frame
   if (prev_regnum == ARM_PS_REGNUM)
     {
       CORE_ADDR lr, cpsr;
+      ULONGEST t_bit = arm_psr_thumb_bit (gdbarch);
 
       cpsr = get_frame_register_unsigned (this_frame, prev_regnum);
       lr = frame_unwind_register_unsigned (this_frame, ARM_LR_REGNUM);
       if (IS_THUMB_ADDR (lr))
-	cpsr |= CPSR_T;
+	cpsr |= t_bit;
       else
-	cpsr &= ~CPSR_T;
+	cpsr &= ~t_bit;
       return frame_unwind_got_constant (this_frame, prev_regnum, cpsr);
     }
 
@@ -1640,6 +1659,7 @@ arm_dwarf2_prev_register (struct frame_i
 {
   struct gdbarch * gdbarch = get_frame_arch (this_frame);
   CORE_ADDR lr, cpsr;
+  ULONGEST t_bit = arm_psr_thumb_bit (gdbarch);
 
   switch (regnum)
     {
@@ -1657,9 +1677,9 @@ arm_dwarf2_prev_register (struct frame_i
       cpsr = get_frame_register_unsigned (this_frame, regnum);
       lr = frame_unwind_register_unsigned (this_frame, ARM_LR_REGNUM);
       if (IS_THUMB_ADDR (lr))
-	cpsr |= CPSR_T;
+	cpsr |= t_bit;
       else
-	cpsr &= ~CPSR_T;
+	cpsr &= ~t_bit;
       return frame_unwind_got_constant (this_frame, regnum, cpsr);
 
     default:
@@ -2008,7 +2028,7 @@ arm_push_dummy_call (struct gdbarch *gdb
 
   /* Set the return address.  For the ARM, the return breakpoint is
      always at BP_ADDR.  */
-  if (arm_pc_is_thumb (bp_addr))
+  if (arm_pc_is_thumb (gdbarch, bp_addr))
     bp_addr |= 1;
   regcache_cooked_write_unsigned (regcache, ARM_LR_REGNUM, bp_addr);
 
@@ -2147,7 +2167,7 @@ arm_push_dummy_call (struct gdbarch *gdb
 	  && TYPE_CODE_FUNC == TYPE_CODE (target_type))
 	{
 	  CORE_ADDR regval = extract_unsigned_integer (val, len, byte_order);
-	  if (arm_pc_is_thumb (regval))
+	  if (arm_pc_is_thumb (gdbarch, regval))
 	    {
 	      bfd_byte *copy = alloca (len);
 	      store_unsigned_integer (copy, len, byte_order,
@@ -3352,7 +3372,7 @@ arm_adjust_breakpoint_address (struct gd
     return bpaddr;
 
   /* ARM mode does not have this problem.  */
-  if (!arm_pc_is_thumb (bpaddr))
+  if (!arm_pc_is_thumb (gdbarch, bpaddr))
     return bpaddr;
 
   /* We are setting a breakpoint in Thumb code that could potentially
@@ -3543,10 +3563,11 @@ static int
 displaced_in_arm_mode (struct regcache *regs)
 {
   ULONGEST ps;
+  ULONGEST t_bit = arm_psr_thumb_bit (get_regcache_arch (regs));
 
   regcache_cooked_read_unsigned (regs, ARM_PS_REGNUM, &ps);
 
-  return (ps & CPSR_T) == 0;
+  return (ps & t_bit) == 0;
 }
 
 /* Write to the PC as from a branch instruction.  */
@@ -3568,18 +3589,18 @@ static void
 bx_write_pc (struct regcache *regs, ULONGEST val)
 {
   ULONGEST ps;
+  ULONGEST t_bit = arm_psr_thumb_bit (get_regcache_arch (regs));
 
   regcache_cooked_read_unsigned (regs, ARM_PS_REGNUM, &ps);
 
   if ((val & 1) == 1)
     {
-      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps | CPSR_T);
+      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps | t_bit);
       regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, val & 0xfffffffe);
     }
   else if ((val & 2) == 0)
     {
-      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM,
-				      ps & ~(ULONGEST) CPSR_T);
+      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps & ~t_bit);
       regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, val);
     }
   else
@@ -3587,8 +3608,7 @@ bx_write_pc (struct regcache *regs, ULON
       /* Unpredictable behaviour.  Try to do something sensible (switch to ARM
 	  mode, align dest to 4 bytes).  */
       warning (_("Single-stepping BX to non-word-aligned ARM instruction."));
-      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM,
-				      ps & ~(ULONGEST) CPSR_T);
+      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps & ~t_bit);
       regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, val & 0xfffffffc);
     }
 }
@@ -5345,7 +5365,9 @@ arm_displaced_step_fixup (struct gdbarch
 static int
 gdb_print_insn_arm (bfd_vma memaddr, disassemble_info *info)
 {
-  if (arm_pc_is_thumb (memaddr))
+  struct gdbarch *gdbarch = info->application_data;
+
+  if (arm_pc_is_thumb (gdbarch, memaddr))
     {
       static asymbol *asym;
       static combined_entry_type ce;
@@ -5435,7 +5457,7 @@ arm_breakpoint_from_pc (struct gdbarch *
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
 
-  if (arm_pc_is_thumb (*pcptr))
+  if (arm_pc_is_thumb (gdbarch, *pcptr))
     {
       *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
 
@@ -5474,7 +5496,7 @@ arm_remote_breakpoint_from_pc (struct gd
 
   arm_breakpoint_from_pc (gdbarch, pcptr, kindptr);
 
-  if (arm_pc_is_thumb (*pcptr) && *kindptr == 4)
+  if (arm_pc_is_thumb (gdbarch, *pcptr) && *kindptr == 4)
     /* The documented magic value for a 32-bit Thumb-2 breakpoint, so
        that this is not confused with a 32-bit ARM breakpoint.  */
     *kindptr = 3;
@@ -6219,18 +6241,21 @@ arm_record_special_symbol (struct gdbarc
 static void
 arm_write_pc (struct regcache *regcache, CORE_ADDR pc)
 {
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
   regcache_cooked_write_unsigned (regcache, ARM_PC_REGNUM, pc);
 
   /* If necessary, set the T bit.  */
   if (arm_apcs_32)
     {
-      ULONGEST val;
+      ULONGEST val, t_bit;
       regcache_cooked_read_unsigned (regcache, ARM_PS_REGNUM, &val);
-      if (arm_pc_is_thumb (pc))
-	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM, val | CPSR_T);
+      t_bit = arm_psr_thumb_bit (gdbarch);
+      if (arm_pc_is_thumb (gdbarch, pc))
+	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
+					val | t_bit);
       else
 	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
-					val & ~(ULONGEST) CPSR_T);
+					val & ~t_bit);
     }
 }
 
@@ -6411,13 +6436,158 @@ arm_gdbarch_init (struct gdbarch_info in
   enum arm_abi_kind arm_abi = arm_abi_global;
   enum arm_float_model fp_model = arm_fp_model;
   struct tdesc_arch_data *tdesc_data = NULL;
-  int i;
+  int i, is_m = 0;
   int have_vfp_registers = 0, have_vfp_pseudos = 0, have_neon_pseudos = 0;
   int have_neon = 0;
   int have_fpa_registers = 1;
+  const struct target_desc *tdesc = info.target_desc;
+
+  /* If we have an object to base this architecture on, try to determine
+     its ABI.  */
+
+  if (arm_abi == ARM_ABI_AUTO && info.abfd != NULL)
+    {
+      int ei_osabi, e_flags;
+
+      switch (bfd_get_flavour (info.abfd))
+	{
+	case bfd_target_aout_flavour:
+	  /* Assume it's an old APCS-style ABI.  */
+	  arm_abi = ARM_ABI_APCS;
+	  break;
+
+	case bfd_target_coff_flavour:
+	  /* Assume it's an old APCS-style ABI.  */
+	  /* XXX WinCE?  */
+	  arm_abi = ARM_ABI_APCS;
+	  break;
+
+	case bfd_target_elf_flavour:
+	  ei_osabi = elf_elfheader (info.abfd)->e_ident[EI_OSABI];
+	  e_flags = elf_elfheader (info.abfd)->e_flags;
+
+	  if (ei_osabi == ELFOSABI_ARM)
+	    {
+	      /* GNU tools used to use this value, but do not for EABI
+		 objects.  There's nowhere to tag an EABI version
+		 anyway, so assume APCS.  */
+	      arm_abi = ARM_ABI_APCS;
+	    }
+	  else if (ei_osabi == ELFOSABI_NONE)
+	    {
+	      int eabi_ver = EF_ARM_EABI_VERSION (e_flags);
+	      int attr;
+
+	      switch (eabi_ver)
+		{
+		case EF_ARM_EABI_UNKNOWN:
+		  /* Assume GNU tools.  */
+		  arm_abi = ARM_ABI_APCS;
+		  break;
+
+		case EF_ARM_EABI_VER4:
+		case EF_ARM_EABI_VER5:
+		  arm_abi = ARM_ABI_AAPCS;
+		  /* EABI binaries default to VFP float ordering.
+		     They may also contain build attributes that can
+		     be used to identify if the VFP argument-passing
+		     ABI is in use.  */
+		  if (fp_model == ARM_FLOAT_AUTO)
+		    {
+#ifdef HAVE_ELF
+		      switch (bfd_elf_get_obj_attr_int (info.abfd,
+							OBJ_ATTR_PROC,
+							Tag_ABI_VFP_args))
+			{
+			case 0:
+			  /* "The user intended FP parameter/result
+			     passing to conform to AAPCS, base
+			     variant".  */
+			  fp_model = ARM_FLOAT_SOFT_VFP;
+			  break;
+			case 1:
+			  /* "The user intended FP parameter/result
+			     passing to conform to AAPCS, VFP
+			     variant".  */
+			  fp_model = ARM_FLOAT_VFP;
+			  break;
+			case 2:
+			  /* "The user intended FP parameter/result
+			     passing to conform to tool chain-specific
+			     conventions" - we don't know any such
+			     conventions, so leave it as "auto".  */
+			  break;
+			default:
+			  /* Attribute value not mentioned in the
+			     October 2008 ABI, so leave it as
+			     "auto".  */
+			  break;
+			}
+#else
+		      fp_model = ARM_FLOAT_SOFT_VFP;
+#endif
+		    }
+		  break;
+
+		default:
+		  /* Leave it as "auto".  */
+		  warning (_("unknown ARM EABI version 0x%x"), eabi_ver);
+		  break;
+		}
+
+#ifdef HAVE_ELF
+	      /* Detect M-profile programs.  */
+	      attr = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
+					       Tag_CPU_arch);
+	      /* The lowest value with a meaningful profile is V7; V6-M
+		 has a higher attribute value.  */
+	      if (attr >= TAG_CPU_ARCH_V7)
+		{
+		  attr = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
+						   Tag_CPU_arch_profile);
+		  if (attr == 'M' && ! tdesc_has_registers (tdesc))
+		    tdesc = tdesc_arm_with_m;
+		}
+#endif
+	    }
+
+	  if (fp_model == ARM_FLOAT_AUTO)
+	    {
+	      int e_flags = elf_elfheader (info.abfd)->e_flags;
+
+	      switch (e_flags & (EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT))
+		{
+		case 0:
+		  /* Leave it as "auto".  Strictly speaking this case
+		     means FPA, but almost nobody uses that now, and
+		     many toolchains fail to set the appropriate bits
+		     for the floating-point model they use.  */
+		  break;
+		case EF_ARM_SOFT_FLOAT:
+		  fp_model = ARM_FLOAT_SOFT_FPA;
+		  break;
+		case EF_ARM_VFP_FLOAT:
+		  fp_model = ARM_FLOAT_VFP;
+		  break;
+		case EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT:
+		  fp_model = ARM_FLOAT_SOFT_VFP;
+		  break;
+		}
+	    }
+
+	  if (e_flags & EF_ARM_BE8)
+	    info.byte_order_for_code = BFD_ENDIAN_LITTLE;
+
+	  break;
+
+	default:
+	  /* Leave it as "auto".  */
+	  break;
+	}
+    }
 
   /* Check any target description for validity.  */
-  if (tdesc_has_registers (info.target_desc))
+  if (tdesc_has_registers (tdesc))
     {
       /* For most registers we require GDB's default names; but also allow
 	 the numeric names for sp / lr / pc, as a convenience.  */
@@ -6428,10 +6598,17 @@ arm_gdbarch_init (struct gdbarch_info in
       const struct tdesc_feature *feature;
       int valid_p;
 
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.arm.core");
       if (feature == NULL)
-	return NULL;
+	{
+	  feature = tdesc_find_feature (tdesc,
+					"org.gnu.gdb.arm.m-profile");
+	  if (feature == NULL)
+	    return NULL;
+	  else
+	    is_m = 1;
+	}
 
       tdesc_data = tdesc_data_alloc ();
 
@@ -6448,8 +6625,12 @@ arm_gdbarch_init (struct gdbarch_info in
       valid_p &= tdesc_numbered_register_choices (feature, tdesc_data,
 						  ARM_PC_REGNUM,
 						  arm_pc_names);
-      valid_p &= tdesc_numbered_register (feature, tdesc_data,
-					  ARM_PS_REGNUM, "cpsr");
+      if (is_m)
+	valid_p &= tdesc_numbered_register (feature, tdesc_data,
+					    ARM_PS_REGNUM, "xpsr");
+      else
+	valid_p &= tdesc_numbered_register (feature, tdesc_data,
+					    ARM_PS_REGNUM, "cpsr");
 
       if (!valid_p)
 	{
@@ -6457,7 +6638,7 @@ arm_gdbarch_init (struct gdbarch_info in
 	  return NULL;
 	}
 
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.arm.fpa");
       if (feature != NULL)
 	{
@@ -6474,7 +6655,7 @@ arm_gdbarch_init (struct gdbarch_info in
       else
 	have_fpa_registers = 0;
 
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.xscale.iwmmxt");
       if (feature != NULL)
 	{
@@ -6512,7 +6693,7 @@ arm_gdbarch_init (struct gdbarch_info in
       /* If we have a VFP unit, check whether the single precision registers
 	 are present.  If not, then we will synthesize them as pseudo
 	 registers.  */
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.arm.vfp");
       if (feature != NULL)
 	{
@@ -6549,7 +6730,7 @@ arm_gdbarch_init (struct gdbarch_info in
 	  /* If we have VFP, also check for NEON.  The architecture allows
 	     NEON without VFP (integer vector operations only), but GDB
 	     does not support that.  */
-	  feature = tdesc_find_feature (info.target_desc,
+	  feature = tdesc_find_feature (tdesc,
 					"org.gnu.gdb.arm.neon");
 	  if (feature != NULL)
 	    {
@@ -6571,134 +6752,6 @@ arm_gdbarch_init (struct gdbarch_info in
 	}
     }
 
-  /* If we have an object to base this architecture on, try to determine
-     its ABI.  */
-
-  if (arm_abi == ARM_ABI_AUTO && info.abfd != NULL)
-    {
-      int ei_osabi, e_flags;
-
-      switch (bfd_get_flavour (info.abfd))
-	{
-	case bfd_target_aout_flavour:
-	  /* Assume it's an old APCS-style ABI.  */
-	  arm_abi = ARM_ABI_APCS;
-	  break;
-
-	case bfd_target_coff_flavour:
-	  /* Assume it's an old APCS-style ABI.  */
-	  /* XXX WinCE?  */
-	  arm_abi = ARM_ABI_APCS;
-	  break;
-
-	case bfd_target_elf_flavour:
-	  ei_osabi = elf_elfheader (info.abfd)->e_ident[EI_OSABI];
-	  e_flags = elf_elfheader (info.abfd)->e_flags;
-
-	  if (ei_osabi == ELFOSABI_ARM)
-	    {
-	      /* GNU tools used to use this value, but do not for EABI
-		 objects.  There's nowhere to tag an EABI version
-		 anyway, so assume APCS.  */
-	      arm_abi = ARM_ABI_APCS;
-	    }
-	  else if (ei_osabi == ELFOSABI_NONE)
-	    {
-	      int eabi_ver = EF_ARM_EABI_VERSION (e_flags);
-
-	      switch (eabi_ver)
-		{
-		case EF_ARM_EABI_UNKNOWN:
-		  /* Assume GNU tools.  */
-		  arm_abi = ARM_ABI_APCS;
-		  break;
-
-		case EF_ARM_EABI_VER4:
-		case EF_ARM_EABI_VER5:
-		  arm_abi = ARM_ABI_AAPCS;
-		  /* EABI binaries default to VFP float ordering.
-		     They may also contain build attributes that can
-		     be used to identify if the VFP argument-passing
-		     ABI is in use.  */
-		  if (fp_model == ARM_FLOAT_AUTO)
-		    {
-#ifdef HAVE_ELF
-		      switch (bfd_elf_get_obj_attr_int (info.abfd,
-							OBJ_ATTR_PROC,
-							Tag_ABI_VFP_args))
-			{
-			case 0:
-			  /* "The user intended FP parameter/result
-			     passing to conform to AAPCS, base
-			     variant".  */
-			  fp_model = ARM_FLOAT_SOFT_VFP;
-			  break;
-			case 1:
-			  /* "The user intended FP parameter/result
-			     passing to conform to AAPCS, VFP
-			     variant".  */
-			  fp_model = ARM_FLOAT_VFP;
-			  break;
-			case 2:
-			  /* "The user intended FP parameter/result
-			     passing to conform to tool chain-specific
-			     conventions" - we don't know any such
-			     conventions, so leave it as "auto".  */
-			  break;
-			default:
-			  /* Attribute value not mentioned in the
-			     October 2008 ABI, so leave it as
-			     "auto".  */
-			  break;
-			}
-#else
-		      fp_model = ARM_FLOAT_SOFT_VFP;
-#endif
-		    }
-		  break;
-
-		default:
-		  /* Leave it as "auto".  */
-		  warning (_("unknown ARM EABI version 0x%x"), eabi_ver);
-		  break;
-		}
-	    }
-
-	  if (fp_model == ARM_FLOAT_AUTO)
-	    {
-	      int e_flags = elf_elfheader (info.abfd)->e_flags;
-
-	      switch (e_flags & (EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT))
-		{
-		case 0:
-		  /* Leave it as "auto".  Strictly speaking this case
-		     means FPA, but almost nobody uses that now, and
-		     many toolchains fail to set the appropriate bits
-		     for the floating-point model they use.  */
-		  break;
-		case EF_ARM_SOFT_FLOAT:
-		  fp_model = ARM_FLOAT_SOFT_FPA;
-		  break;
-		case EF_ARM_VFP_FLOAT:
-		  fp_model = ARM_FLOAT_VFP;
-		  break;
-		case EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT:
-		  fp_model = ARM_FLOAT_SOFT_VFP;
-		  break;
-		}
-	    }
-
-	  if (e_flags & EF_ARM_BE8)
-	    info.byte_order_for_code = BFD_ENDIAN_LITTLE;
-
-	  break;
-
-	default:
-	  /* Leave it as "auto".  */
-	  break;
-	}
-    }
-
   /* If there is already a candidate, use it.  */
   for (best_arch = gdbarch_list_lookup_by_info (arches, &info);
        best_arch != NULL;
@@ -6717,6 +6770,10 @@ arm_gdbarch_init (struct gdbarch_info in
 	 since gdbarches with a different target description are
 	 automatically disqualified.  */
 
+      /* Do check is_m, though, since it might come from the binary.  */
+      if (is_m != gdbarch_tdep (best_arch->gdbarch)->is_m)
+	continue;
+
       /* Found a match.  */
       break;
     }
@@ -6735,6 +6792,7 @@ arm_gdbarch_init (struct gdbarch_info in
      These are gdbarch discriminators, like the OSABI.  */
   tdep->arm_abi = arm_abi;
   tdep->fp_model = fp_model;
+  tdep->is_m = is_m;
   tdep->have_fpa_registers = have_fpa_registers;
   tdep->have_vfp_registers = have_vfp_registers;
   tdep->have_vfp_pseudos = have_vfp_pseudos;
@@ -6908,7 +6966,7 @@ arm_gdbarch_init (struct gdbarch_info in
     {
       set_tdesc_pseudo_register_name (gdbarch, arm_register_name);
 
-      tdesc_use_registers (gdbarch, info.target_desc, tdesc_data);
+      tdesc_use_registers (gdbarch, tdesc, tdesc_data);
 
       /* Override tdesc_register_type to adjust the types of VFP
 	 registers for NEON.  */
@@ -6963,6 +7021,9 @@ _initialize_arm_tdep (void)
 				  bfd_target_elf_flavour,
 				  arm_elf_osabi_sniffer);
 
+  /* Initialize the standard target descriptions.  */
+  initialize_tdesc_arm_with_m ();
+
   /* Get the number of possible sets of register names defined in opcodes.  */
   num_disassembly_options = get_arm_regname_num_options ();
 
Index: arm-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.h,v
retrieving revision 1.40
diff -u -p -r1.40 arm-tdep.h
--- arm-tdep.h	12 Apr 2010 13:52:43 -0000	1.40
+++ arm-tdep.h	16 Aug 2010 16:00:57 -0000
@@ -108,6 +108,8 @@ enum gdb_regnum {
 
 #define CPSR_T		0x20
 
+#define XPSR_T		0x01000000
+
 /* Type of floating-point code in use by inferior.  There are really 3 models
    that are traditionally supported (plus the endianness issue), but gcc can
    only generate 2 of those.  The third is APCS_FLOAT, where arguments to
@@ -163,6 +165,7 @@ struct gdbarch_tdep
 				   have_vfp_pseudos.  */
   int have_neon;		/* Do we have a NEON unit?  */
 
+  int is_m;			/* Does the target follow the "M" profile.  */
   CORE_ADDR lowest_pc;		/* Lowest address at which instructions 
 				   will appear.  */
 
Index: features/arm-m-profile.xml
===================================================================
RCS file: features/arm-m-profile.xml
diff -N features/arm-m-profile.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-m-profile.xml	16 Aug 2010 16:00:58 -0000
@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.arm.m-profile">
+  <reg name="r0" bitsize="32"/>
+  <reg name="r1" bitsize="32"/>
+  <reg name="r2" bitsize="32"/>
+  <reg name="r3" bitsize="32"/>
+  <reg name="r4" bitsize="32"/>
+  <reg name="r5" bitsize="32"/>
+  <reg name="r6" bitsize="32"/>
+  <reg name="r7" bitsize="32"/>
+  <reg name="r8" bitsize="32"/>
+  <reg name="r9" bitsize="32"/>
+  <reg name="r10" bitsize="32"/>
+  <reg name="r11" bitsize="32"/>
+  <reg name="r12" bitsize="32"/>
+  <reg name="sp" bitsize="32" type="data_ptr"/>
+  <reg name="lr" bitsize="32"/>
+  <reg name="pc" bitsize="32" type="code_ptr"/>
+  <reg name="xpsr" bitsize="32" regnum="25"/>
+</feature>
Index: features/arm-with-m.c
===================================================================
RCS file: features/arm-with-m.c
diff -N features/arm-with-m.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m.c	16 Aug 2010 16:00:58 -0000
@@ -0,0 +1,35 @@
+/* THIS FILE IS GENERATED.  Original: arm-with-m.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arm_with_m;
+static void
+initialize_tdesc_arm_with_m (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+  struct tdesc_type *field_type, *type;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "sp", 13, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "lr", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "pc", 15, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
+
+  tdesc_arm_with_m = result;
+}
Index: features/arm-with-m.xml
===================================================================
RCS file: features/arm-with-m.xml
diff -N features/arm-with-m.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m.xml	16 Aug 2010 16:00:58 -0000
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <xi:include href="arm-m-profile.xml"/>
+</target>
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.754
diff -u -p -r1.754 gdb.texinfo
--- doc/gdb.texinfo	13 Aug 2010 20:39:09 -0000	1.754
+++ doc/gdb.texinfo	16 Aug 2010 18:00:40 -0000
@@ -35606,10 +35606,16 @@ registers using the capitalization used 
 @subsection ARM Features
 @cindex target descriptions, ARM features
 
-The @samp{org.gnu.gdb.arm.core} feature is required for ARM targets.
+The @samp{org.gnu.gdb.arm.core} feature is required for non-M-profile
+ARM targets.
 It should contain registers @samp{r0} through @samp{r13}, @samp{sp},
 @samp{lr}, @samp{pc}, and @samp{cpsr}.
 
+For M-profile targets (e.g. Cortex-M3), the @samp{org.gnu.gdb.arm.core}
+feature is replaced by @samp{org.gnu.gdb.arm.m-profile}.  It should contain
+registers @samp{r0} through @samp{r13}, @samp{sp}, @samp{lr}, @samp{pc},
+and @samp{xpsr}.
+
 The @samp{org.gnu.gdb.arm.fpa} feature is optional.  If present, it
 should contain registers @samp{f0} through @samp{f7} and @samp{fps}.
 
`

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-08-16 18:06       ` Daniel Jacobowitz
@ 2010-08-17  9:00         ` Matthew Gretton-Dann
  2010-08-24 15:56           ` Daniel Jacobowitz
  2010-10-29 23:47         ` Jonathan Larmour
  1 sibling, 1 reply; 14+ messages in thread
From: Matthew Gretton-Dann @ 2010-08-17  9:00 UTC (permalink / raw)
  To: Daniel Jacobowitz
  Cc: Richard Earnshaw, Kazu Hirata, gdb-patches, Jonathan Larmour

Daniel,

In arm_gdbarch_init you have the following:

On Mon, 2010-08-16 at 14:05 -0400, Daniel Jacobowitz wrote:
> +#ifdef HAVE_ELF
> +	      /* Detect M-profile programs.  */
> +	      attr = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
> +					       Tag_CPU_arch);
> +	      /* The lowest value with a meaningful profile is V7; V6-M
> +		 has a higher attribute value.  */
> +	      if (attr >= TAG_CPU_ARCH_V7)
> +		{
> +		  attr = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
> +						   Tag_CPU_arch_profile);
> +		  if (attr == 'M' && ! tdesc_has_registers (tdesc))
> +		    tdesc = tdesc_arm_with_m;
> +		}
> +#endif

Some toolchains do not output Tag_CPU_arch_profile for v6-M or v6S-M (I
think they are taking the viewthat v6-M is before v7 and so
Tag_CPU_arch_profile does not apply).  So this part of the patch will
not pick up images built for v6-M or v6S-M by those toolchains.  

You probably need an explicit test for Tag_CPU_arch being
TAG_CPU_ARCH_V6_M or TAG_CPU_ARCH_V6S_M and if it is then forcing
M-profile at the point (as well as the full test for architecture v7 and
above).

Otherwise it looks good to me.

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-08-17  9:00         ` Matthew Gretton-Dann
@ 2010-08-24 15:56           ` Daniel Jacobowitz
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Jacobowitz @ 2010-08-24 15:56 UTC (permalink / raw)
  To: Matthew Gretton-Dann
  Cc: Richard Earnshaw, Kazu Hirata, gdb-patches, Jonathan Larmour

On Tue, Aug 17, 2010 at 09:59:27AM +0100, Matthew Gretton-Dann wrote:
> Daniel,
> 
> In arm_gdbarch_init you have the following:
> 
> On Mon, 2010-08-16 at 14:05 -0400, Daniel Jacobowitz wrote:
> > +#ifdef HAVE_ELF
> > +	      /* Detect M-profile programs.  */
> > +	      attr = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
> > +					       Tag_CPU_arch);
> > +	      /* The lowest value with a meaningful profile is V7; V6-M
> > +		 has a higher attribute value.  */
> > +	      if (attr >= TAG_CPU_ARCH_V7)
> > +		{
> > +		  attr = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
> > +						   Tag_CPU_arch_profile);
> > +		  if (attr == 'M' && ! tdesc_has_registers (tdesc))
> > +		    tdesc = tdesc_arm_with_m;
> > +		}
> > +#endif
> 
> Some toolchains do not output Tag_CPU_arch_profile for v6-M or v6S-M (I
> think they are taking the viewthat v6-M is before v7 and so
> Tag_CPU_arch_profile does not apply).  So this part of the patch will
> not pick up images built for v6-M or v6S-M by those toolchains.  
> 
> You probably need an explicit test for Tag_CPU_arch being
> TAG_CPU_ARCH_V6_M or TAG_CPU_ARCH_V6S_M and if it is then forcing
> M-profile at the point (as well as the full test for architecture v7 and
> above).

Some toolchains, in this case, including RealView.  But that's
actually moot; RealView does not propagate build attributes into
executables.  So GDB won't find them at all.

I've checked in this version with that change and an additional note.

-- 
Daniel Jacobowitz
CodeSourcery

2010-08-24  Daniel Jacobowitz  <dan@codesourcery.com>
	    Kazu Hirata  <kazu@codesourcery.com>
	    Jonathan Larmour  <jifl@eCosCentric.com>

	* arm-tdep.c: Include features/arm-with-m.c.
	(arm_psr_thumb_bit): New.  Update all uses of CPSR_T to
	call this function.
	(arm_pc_is_thumb): Add a gdbarch argument.  Update all callers.
	Check is_m after force-mode.
	(arm_gdbarch_init): Check the binary before the target description.
	Add check for M profile attribute.  If we have an M-profile device,
	but no target register description, use arm-with-m.  Recognize the
	new org.gnu.gdb.arm.m-profile feature and its xpsr register.
	(_initialize_arm_tdep): Call initialize_tdesc_arm_with_m.
	* arm-tdep.h (XPSR_T): Define.
	(struct gdbarch_tdep): Add is_m member.
	* features/arm-m-profile.xml, features/arm-with-m.c,
	features/arm-with-m.xml: New files.

	* gdb.texinfo (ARM Features): Document
	org.gnu.gdb.arm.m-profile.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.305
diff -u -p -r1.305 arm-tdep.c
--- arm-tdep.c	14 Aug 2010 00:45:39 -0000	1.305
+++ arm-tdep.c	24 Aug 2010 15:47:05 -0000
@@ -53,6 +53,8 @@
 #include "gdb_assert.h"
 #include "vec.h"
 
+#include "features/arm-with-m.c"
+
 static int arm_debug;
 
 /* Macros for setting and testing a bit in a minimal symbol that marks
@@ -255,12 +257,24 @@ static CORE_ADDR arm_analyze_prologue (s
 
 int arm_apcs_32 = 1;
 
+/* Return the bit mask in ARM_PS_REGNUM that indicates Thumb mode.  */
+
+static int
+arm_psr_thumb_bit (struct gdbarch *gdbarch)
+{
+  if (gdbarch_tdep (gdbarch)->is_m)
+    return XPSR_T;
+  else
+    return CPSR_T;
+}
+
 /* Determine if FRAME is executing in Thumb mode.  */
 
 static int
 arm_frame_is_thumb (struct frame_info *frame)
 {
   CORE_ADDR cpsr;
+  ULONGEST t_bit = arm_psr_thumb_bit (get_frame_arch (frame));
 
   /* Every ARM frame unwinder can unwind the T bit of the CPSR, either
      directly (from a signal frame or dummy frame) or by interpreting
@@ -268,7 +282,7 @@ arm_frame_is_thumb (struct frame_info *f
      trust the unwinders.  */
   cpsr = get_frame_register_unsigned (frame, ARM_PS_REGNUM);
 
-  return (cpsr & CPSR_T) != 0;
+  return (cpsr & t_bit) != 0;
 }
 
 /* Callback for VEC_lower_bound.  */
@@ -347,7 +361,7 @@ static CORE_ADDR arm_get_next_pc_raw (st
    any executing frame; otherwise, prefer arm_frame_is_thumb.  */
 
 static int
-arm_pc_is_thumb (CORE_ADDR memaddr)
+arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr)
 {
   struct obj_section *sec;
   struct minimal_symbol *sym;
@@ -363,6 +377,10 @@ arm_pc_is_thumb (CORE_ADDR memaddr)
   if (strcmp (arm_force_mode_string, "thumb") == 0)
     return 1;
 
+  /* ARM v6-M and v7-M are always in Thumb mode.  */
+  if (gdbarch_tdep (gdbarch)->is_m)
+    return 1;
+
   /* If there are mapping symbols, consult them.  */
   type = arm_find_mapping_symbol (memaddr, NULL);
   if (type)
@@ -815,7 +833,7 @@ arm_skip_prologue (struct gdbarch *gdbar
 	     associate prologue code with the opening brace; so this
 	     lets us skip the first line if we think it is the opening
 	     brace.  */
-	  if (arm_pc_is_thumb (func_addr))
+	  if (arm_pc_is_thumb (gdbarch, func_addr))
 	    analyzed_limit = thumb_analyze_prologue (gdbarch, func_addr,
 						     post_prologue_pc, NULL);
 	  else
@@ -842,7 +860,7 @@ arm_skip_prologue (struct gdbarch *gdbar
 
 
   /* Check if this is Thumb code.  */
-  if (arm_pc_is_thumb (pc))
+  if (arm_pc_is_thumb (gdbarch, pc))
     return thumb_analyze_prologue (gdbarch, pc, limit_pc, NULL);
 
   for (skip_pc = pc; skip_pc < limit_pc; skip_pc += 4)
@@ -1507,13 +1525,14 @@ arm_prologue_prev_register (struct frame
   if (prev_regnum == ARM_PS_REGNUM)
     {
       CORE_ADDR lr, cpsr;
+      ULONGEST t_bit = arm_psr_thumb_bit (gdbarch);
 
       cpsr = get_frame_register_unsigned (this_frame, prev_regnum);
       lr = frame_unwind_register_unsigned (this_frame, ARM_LR_REGNUM);
       if (IS_THUMB_ADDR (lr))
-	cpsr |= CPSR_T;
+	cpsr |= t_bit;
       else
-	cpsr &= ~CPSR_T;
+	cpsr &= ~t_bit;
       return frame_unwind_got_constant (this_frame, prev_regnum, cpsr);
     }
 
@@ -1640,6 +1659,7 @@ arm_dwarf2_prev_register (struct frame_i
 {
   struct gdbarch * gdbarch = get_frame_arch (this_frame);
   CORE_ADDR lr, cpsr;
+  ULONGEST t_bit = arm_psr_thumb_bit (gdbarch);
 
   switch (regnum)
     {
@@ -1657,9 +1677,9 @@ arm_dwarf2_prev_register (struct frame_i
       cpsr = get_frame_register_unsigned (this_frame, regnum);
       lr = frame_unwind_register_unsigned (this_frame, ARM_LR_REGNUM);
       if (IS_THUMB_ADDR (lr))
-	cpsr |= CPSR_T;
+	cpsr |= t_bit;
       else
-	cpsr &= ~CPSR_T;
+	cpsr &= ~t_bit;
       return frame_unwind_got_constant (this_frame, regnum, cpsr);
 
     default:
@@ -2008,7 +2028,7 @@ arm_push_dummy_call (struct gdbarch *gdb
 
   /* Set the return address.  For the ARM, the return breakpoint is
      always at BP_ADDR.  */
-  if (arm_pc_is_thumb (bp_addr))
+  if (arm_pc_is_thumb (gdbarch, bp_addr))
     bp_addr |= 1;
   regcache_cooked_write_unsigned (regcache, ARM_LR_REGNUM, bp_addr);
 
@@ -2147,7 +2167,7 @@ arm_push_dummy_call (struct gdbarch *gdb
 	  && TYPE_CODE_FUNC == TYPE_CODE (check_typedef (target_type)))
 	{
 	  CORE_ADDR regval = extract_unsigned_integer (val, len, byte_order);
-	  if (arm_pc_is_thumb (regval))
+	  if (arm_pc_is_thumb (gdbarch, regval))
 	    {
 	      bfd_byte *copy = alloca (len);
 	      store_unsigned_integer (copy, len, byte_order,
@@ -3352,7 +3372,7 @@ arm_adjust_breakpoint_address (struct gd
     return bpaddr;
 
   /* ARM mode does not have this problem.  */
-  if (!arm_pc_is_thumb (bpaddr))
+  if (!arm_pc_is_thumb (gdbarch, bpaddr))
     return bpaddr;
 
   /* We are setting a breakpoint in Thumb code that could potentially
@@ -3543,10 +3563,11 @@ static int
 displaced_in_arm_mode (struct regcache *regs)
 {
   ULONGEST ps;
+  ULONGEST t_bit = arm_psr_thumb_bit (get_regcache_arch (regs));
 
   regcache_cooked_read_unsigned (regs, ARM_PS_REGNUM, &ps);
 
-  return (ps & CPSR_T) == 0;
+  return (ps & t_bit) == 0;
 }
 
 /* Write to the PC as from a branch instruction.  */
@@ -3568,18 +3589,18 @@ static void
 bx_write_pc (struct regcache *regs, ULONGEST val)
 {
   ULONGEST ps;
+  ULONGEST t_bit = arm_psr_thumb_bit (get_regcache_arch (regs));
 
   regcache_cooked_read_unsigned (regs, ARM_PS_REGNUM, &ps);
 
   if ((val & 1) == 1)
     {
-      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps | CPSR_T);
+      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps | t_bit);
       regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, val & 0xfffffffe);
     }
   else if ((val & 2) == 0)
     {
-      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM,
-				      ps & ~(ULONGEST) CPSR_T);
+      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps & ~t_bit);
       regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, val);
     }
   else
@@ -3587,8 +3608,7 @@ bx_write_pc (struct regcache *regs, ULON
       /* Unpredictable behaviour.  Try to do something sensible (switch to ARM
 	  mode, align dest to 4 bytes).  */
       warning (_("Single-stepping BX to non-word-aligned ARM instruction."));
-      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM,
-				      ps & ~(ULONGEST) CPSR_T);
+      regcache_cooked_write_unsigned (regs, ARM_PS_REGNUM, ps & ~t_bit);
       regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, val & 0xfffffffc);
     }
 }
@@ -5345,7 +5365,9 @@ arm_displaced_step_fixup (struct gdbarch
 static int
 gdb_print_insn_arm (bfd_vma memaddr, disassemble_info *info)
 {
-  if (arm_pc_is_thumb (memaddr))
+  struct gdbarch *gdbarch = info->application_data;
+
+  if (arm_pc_is_thumb (gdbarch, memaddr))
     {
       static asymbol *asym;
       static combined_entry_type ce;
@@ -5435,7 +5457,7 @@ arm_breakpoint_from_pc (struct gdbarch *
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
 
-  if (arm_pc_is_thumb (*pcptr))
+  if (arm_pc_is_thumb (gdbarch, *pcptr))
     {
       *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
 
@@ -5474,7 +5496,7 @@ arm_remote_breakpoint_from_pc (struct gd
 
   arm_breakpoint_from_pc (gdbarch, pcptr, kindptr);
 
-  if (arm_pc_is_thumb (*pcptr) && *kindptr == 4)
+  if (arm_pc_is_thumb (gdbarch, *pcptr) && *kindptr == 4)
     /* The documented magic value for a 32-bit Thumb-2 breakpoint, so
        that this is not confused with a 32-bit ARM breakpoint.  */
     *kindptr = 3;
@@ -6219,18 +6241,21 @@ arm_record_special_symbol (struct gdbarc
 static void
 arm_write_pc (struct regcache *regcache, CORE_ADDR pc)
 {
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
   regcache_cooked_write_unsigned (regcache, ARM_PC_REGNUM, pc);
 
   /* If necessary, set the T bit.  */
   if (arm_apcs_32)
     {
-      ULONGEST val;
+      ULONGEST val, t_bit;
       regcache_cooked_read_unsigned (regcache, ARM_PS_REGNUM, &val);
-      if (arm_pc_is_thumb (pc))
-	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM, val | CPSR_T);
+      t_bit = arm_psr_thumb_bit (gdbarch);
+      if (arm_pc_is_thumb (gdbarch, pc))
+	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
+					val | t_bit);
       else
 	regcache_cooked_write_unsigned (regcache, ARM_PS_REGNUM,
-					val & ~(ULONGEST) CPSR_T);
+					val & ~t_bit);
     }
 }
 
@@ -6411,13 +6436,163 @@ arm_gdbarch_init (struct gdbarch_info in
   enum arm_abi_kind arm_abi = arm_abi_global;
   enum arm_float_model fp_model = arm_fp_model;
   struct tdesc_arch_data *tdesc_data = NULL;
-  int i;
+  int i, is_m = 0;
   int have_vfp_registers = 0, have_vfp_pseudos = 0, have_neon_pseudos = 0;
   int have_neon = 0;
   int have_fpa_registers = 1;
+  const struct target_desc *tdesc = info.target_desc;
+
+  /* If we have an object to base this architecture on, try to determine
+     its ABI.  */
+
+  if (arm_abi == ARM_ABI_AUTO && info.abfd != NULL)
+    {
+      int ei_osabi, e_flags;
+
+      switch (bfd_get_flavour (info.abfd))
+	{
+	case bfd_target_aout_flavour:
+	  /* Assume it's an old APCS-style ABI.  */
+	  arm_abi = ARM_ABI_APCS;
+	  break;
+
+	case bfd_target_coff_flavour:
+	  /* Assume it's an old APCS-style ABI.  */
+	  /* XXX WinCE?  */
+	  arm_abi = ARM_ABI_APCS;
+	  break;
+
+	case bfd_target_elf_flavour:
+	  ei_osabi = elf_elfheader (info.abfd)->e_ident[EI_OSABI];
+	  e_flags = elf_elfheader (info.abfd)->e_flags;
+
+	  if (ei_osabi == ELFOSABI_ARM)
+	    {
+	      /* GNU tools used to use this value, but do not for EABI
+		 objects.  There's nowhere to tag an EABI version
+		 anyway, so assume APCS.  */
+	      arm_abi = ARM_ABI_APCS;
+	    }
+	  else if (ei_osabi == ELFOSABI_NONE)
+	    {
+	      int eabi_ver = EF_ARM_EABI_VERSION (e_flags);
+	      int attr_arch, attr_profile;
+
+	      switch (eabi_ver)
+		{
+		case EF_ARM_EABI_UNKNOWN:
+		  /* Assume GNU tools.  */
+		  arm_abi = ARM_ABI_APCS;
+		  break;
+
+		case EF_ARM_EABI_VER4:
+		case EF_ARM_EABI_VER5:
+		  arm_abi = ARM_ABI_AAPCS;
+		  /* EABI binaries default to VFP float ordering.
+		     They may also contain build attributes that can
+		     be used to identify if the VFP argument-passing
+		     ABI is in use.  */
+		  if (fp_model == ARM_FLOAT_AUTO)
+		    {
+#ifdef HAVE_ELF
+		      switch (bfd_elf_get_obj_attr_int (info.abfd,
+							OBJ_ATTR_PROC,
+							Tag_ABI_VFP_args))
+			{
+			case 0:
+			  /* "The user intended FP parameter/result
+			     passing to conform to AAPCS, base
+			     variant".  */
+			  fp_model = ARM_FLOAT_SOFT_VFP;
+			  break;
+			case 1:
+			  /* "The user intended FP parameter/result
+			     passing to conform to AAPCS, VFP
+			     variant".  */
+			  fp_model = ARM_FLOAT_VFP;
+			  break;
+			case 2:
+			  /* "The user intended FP parameter/result
+			     passing to conform to tool chain-specific
+			     conventions" - we don't know any such
+			     conventions, so leave it as "auto".  */
+			  break;
+			default:
+			  /* Attribute value not mentioned in the
+			     October 2008 ABI, so leave it as
+			     "auto".  */
+			  break;
+			}
+#else
+		      fp_model = ARM_FLOAT_SOFT_VFP;
+#endif
+		    }
+		  break;
+
+		default:
+		  /* Leave it as "auto".  */
+		  warning (_("unknown ARM EABI version 0x%x"), eabi_ver);
+		  break;
+		}
+
+#ifdef HAVE_ELF
+	      /* Detect M-profile programs.  This only works if the
+		 executable file includes build attributes; GCC does
+		 copy them to the executable, but e.g. RealView does
+		 not.  */
+	      attr_arch = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
+						    Tag_CPU_arch);
+	      attr_profile = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
+						       Tag_CPU_arch_profile);
+	      /* GCC specifies the profile for v6-M; RealView only
+		 specifies the profile for architectures starting with
+		 V7 (as opposed to architectures with a tag
+		 numerically greater than TAG_CPU_ARCH_V7).  */
+	      if (!tdesc_has_registers (tdesc)
+		  && (attr_arch == TAG_CPU_ARCH_V6_M
+		      || attr_arch == TAG_CPU_ARCH_V6S_M
+		      || attr_profile == 'M'))
+		tdesc = tdesc_arm_with_m;
+#endif
+	    }
+
+	  if (fp_model == ARM_FLOAT_AUTO)
+	    {
+	      int e_flags = elf_elfheader (info.abfd)->e_flags;
+
+	      switch (e_flags & (EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT))
+		{
+		case 0:
+		  /* Leave it as "auto".  Strictly speaking this case
+		     means FPA, but almost nobody uses that now, and
+		     many toolchains fail to set the appropriate bits
+		     for the floating-point model they use.  */
+		  break;
+		case EF_ARM_SOFT_FLOAT:
+		  fp_model = ARM_FLOAT_SOFT_FPA;
+		  break;
+		case EF_ARM_VFP_FLOAT:
+		  fp_model = ARM_FLOAT_VFP;
+		  break;
+		case EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT:
+		  fp_model = ARM_FLOAT_SOFT_VFP;
+		  break;
+		}
+	    }
+
+	  if (e_flags & EF_ARM_BE8)
+	    info.byte_order_for_code = BFD_ENDIAN_LITTLE;
+
+	  break;
+
+	default:
+	  /* Leave it as "auto".  */
+	  break;
+	}
+    }
 
   /* Check any target description for validity.  */
-  if (tdesc_has_registers (info.target_desc))
+  if (tdesc_has_registers (tdesc))
     {
       /* For most registers we require GDB's default names; but also allow
 	 the numeric names for sp / lr / pc, as a convenience.  */
@@ -6428,10 +6603,17 @@ arm_gdbarch_init (struct gdbarch_info in
       const struct tdesc_feature *feature;
       int valid_p;
 
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.arm.core");
       if (feature == NULL)
-	return NULL;
+	{
+	  feature = tdesc_find_feature (tdesc,
+					"org.gnu.gdb.arm.m-profile");
+	  if (feature == NULL)
+	    return NULL;
+	  else
+	    is_m = 1;
+	}
 
       tdesc_data = tdesc_data_alloc ();
 
@@ -6448,8 +6630,12 @@ arm_gdbarch_init (struct gdbarch_info in
       valid_p &= tdesc_numbered_register_choices (feature, tdesc_data,
 						  ARM_PC_REGNUM,
 						  arm_pc_names);
-      valid_p &= tdesc_numbered_register (feature, tdesc_data,
-					  ARM_PS_REGNUM, "cpsr");
+      if (is_m)
+	valid_p &= tdesc_numbered_register (feature, tdesc_data,
+					    ARM_PS_REGNUM, "xpsr");
+      else
+	valid_p &= tdesc_numbered_register (feature, tdesc_data,
+					    ARM_PS_REGNUM, "cpsr");
 
       if (!valid_p)
 	{
@@ -6457,7 +6643,7 @@ arm_gdbarch_init (struct gdbarch_info in
 	  return NULL;
 	}
 
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.arm.fpa");
       if (feature != NULL)
 	{
@@ -6474,7 +6660,7 @@ arm_gdbarch_init (struct gdbarch_info in
       else
 	have_fpa_registers = 0;
 
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.xscale.iwmmxt");
       if (feature != NULL)
 	{
@@ -6512,7 +6698,7 @@ arm_gdbarch_init (struct gdbarch_info in
       /* If we have a VFP unit, check whether the single precision registers
 	 are present.  If not, then we will synthesize them as pseudo
 	 registers.  */
-      feature = tdesc_find_feature (info.target_desc,
+      feature = tdesc_find_feature (tdesc,
 				    "org.gnu.gdb.arm.vfp");
       if (feature != NULL)
 	{
@@ -6549,7 +6735,7 @@ arm_gdbarch_init (struct gdbarch_info in
 	  /* If we have VFP, also check for NEON.  The architecture allows
 	     NEON without VFP (integer vector operations only), but GDB
 	     does not support that.  */
-	  feature = tdesc_find_feature (info.target_desc,
+	  feature = tdesc_find_feature (tdesc,
 					"org.gnu.gdb.arm.neon");
 	  if (feature != NULL)
 	    {
@@ -6571,134 +6757,6 @@ arm_gdbarch_init (struct gdbarch_info in
 	}
     }
 
-  /* If we have an object to base this architecture on, try to determine
-     its ABI.  */
-
-  if (arm_abi == ARM_ABI_AUTO && info.abfd != NULL)
-    {
-      int ei_osabi, e_flags;
-
-      switch (bfd_get_flavour (info.abfd))
-	{
-	case bfd_target_aout_flavour:
-	  /* Assume it's an old APCS-style ABI.  */
-	  arm_abi = ARM_ABI_APCS;
-	  break;
-
-	case bfd_target_coff_flavour:
-	  /* Assume it's an old APCS-style ABI.  */
-	  /* XXX WinCE?  */
-	  arm_abi = ARM_ABI_APCS;
-	  break;
-
-	case bfd_target_elf_flavour:
-	  ei_osabi = elf_elfheader (info.abfd)->e_ident[EI_OSABI];
-	  e_flags = elf_elfheader (info.abfd)->e_flags;
-
-	  if (ei_osabi == ELFOSABI_ARM)
-	    {
-	      /* GNU tools used to use this value, but do not for EABI
-		 objects.  There's nowhere to tag an EABI version
-		 anyway, so assume APCS.  */
-	      arm_abi = ARM_ABI_APCS;
-	    }
-	  else if (ei_osabi == ELFOSABI_NONE)
-	    {
-	      int eabi_ver = EF_ARM_EABI_VERSION (e_flags);
-
-	      switch (eabi_ver)
-		{
-		case EF_ARM_EABI_UNKNOWN:
-		  /* Assume GNU tools.  */
-		  arm_abi = ARM_ABI_APCS;
-		  break;
-
-		case EF_ARM_EABI_VER4:
-		case EF_ARM_EABI_VER5:
-		  arm_abi = ARM_ABI_AAPCS;
-		  /* EABI binaries default to VFP float ordering.
-		     They may also contain build attributes that can
-		     be used to identify if the VFP argument-passing
-		     ABI is in use.  */
-		  if (fp_model == ARM_FLOAT_AUTO)
-		    {
-#ifdef HAVE_ELF
-		      switch (bfd_elf_get_obj_attr_int (info.abfd,
-							OBJ_ATTR_PROC,
-							Tag_ABI_VFP_args))
-			{
-			case 0:
-			  /* "The user intended FP parameter/result
-			     passing to conform to AAPCS, base
-			     variant".  */
-			  fp_model = ARM_FLOAT_SOFT_VFP;
-			  break;
-			case 1:
-			  /* "The user intended FP parameter/result
-			     passing to conform to AAPCS, VFP
-			     variant".  */
-			  fp_model = ARM_FLOAT_VFP;
-			  break;
-			case 2:
-			  /* "The user intended FP parameter/result
-			     passing to conform to tool chain-specific
-			     conventions" - we don't know any such
-			     conventions, so leave it as "auto".  */
-			  break;
-			default:
-			  /* Attribute value not mentioned in the
-			     October 2008 ABI, so leave it as
-			     "auto".  */
-			  break;
-			}
-#else
-		      fp_model = ARM_FLOAT_SOFT_VFP;
-#endif
-		    }
-		  break;
-
-		default:
-		  /* Leave it as "auto".  */
-		  warning (_("unknown ARM EABI version 0x%x"), eabi_ver);
-		  break;
-		}
-	    }
-
-	  if (fp_model == ARM_FLOAT_AUTO)
-	    {
-	      int e_flags = elf_elfheader (info.abfd)->e_flags;
-
-	      switch (e_flags & (EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT))
-		{
-		case 0:
-		  /* Leave it as "auto".  Strictly speaking this case
-		     means FPA, but almost nobody uses that now, and
-		     many toolchains fail to set the appropriate bits
-		     for the floating-point model they use.  */
-		  break;
-		case EF_ARM_SOFT_FLOAT:
-		  fp_model = ARM_FLOAT_SOFT_FPA;
-		  break;
-		case EF_ARM_VFP_FLOAT:
-		  fp_model = ARM_FLOAT_VFP;
-		  break;
-		case EF_ARM_SOFT_FLOAT | EF_ARM_VFP_FLOAT:
-		  fp_model = ARM_FLOAT_SOFT_VFP;
-		  break;
-		}
-	    }
-
-	  if (e_flags & EF_ARM_BE8)
-	    info.byte_order_for_code = BFD_ENDIAN_LITTLE;
-
-	  break;
-
-	default:
-	  /* Leave it as "auto".  */
-	  break;
-	}
-    }
-
   /* If there is already a candidate, use it.  */
   for (best_arch = gdbarch_list_lookup_by_info (arches, &info);
        best_arch != NULL;
@@ -6717,6 +6775,10 @@ arm_gdbarch_init (struct gdbarch_info in
 	 since gdbarches with a different target description are
 	 automatically disqualified.  */
 
+      /* Do check is_m, though, since it might come from the binary.  */
+      if (is_m != gdbarch_tdep (best_arch->gdbarch)->is_m)
+	continue;
+
       /* Found a match.  */
       break;
     }
@@ -6735,6 +6797,7 @@ arm_gdbarch_init (struct gdbarch_info in
      These are gdbarch discriminators, like the OSABI.  */
   tdep->arm_abi = arm_abi;
   tdep->fp_model = fp_model;
+  tdep->is_m = is_m;
   tdep->have_fpa_registers = have_fpa_registers;
   tdep->have_vfp_registers = have_vfp_registers;
   tdep->have_vfp_pseudos = have_vfp_pseudos;
@@ -6908,7 +6971,7 @@ arm_gdbarch_init (struct gdbarch_info in
     {
       set_tdesc_pseudo_register_name (gdbarch, arm_register_name);
 
-      tdesc_use_registers (gdbarch, info.target_desc, tdesc_data);
+      tdesc_use_registers (gdbarch, tdesc, tdesc_data);
 
       /* Override tdesc_register_type to adjust the types of VFP
 	 registers for NEON.  */
@@ -6963,6 +7026,9 @@ _initialize_arm_tdep (void)
 				  bfd_target_elf_flavour,
 				  arm_elf_osabi_sniffer);
 
+  /* Initialize the standard target descriptions.  */
+  initialize_tdesc_arm_with_m ();
+
   /* Get the number of possible sets of register names defined in opcodes.  */
   num_disassembly_options = get_arm_regname_num_options ();
 
Index: arm-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.h,v
retrieving revision 1.40
diff -u -p -r1.40 arm-tdep.h
--- arm-tdep.h	12 Apr 2010 13:52:43 -0000	1.40
+++ arm-tdep.h	24 Aug 2010 15:47:05 -0000
@@ -108,6 +108,8 @@ enum gdb_regnum {
 
 #define CPSR_T		0x20
 
+#define XPSR_T		0x01000000
+
 /* Type of floating-point code in use by inferior.  There are really 3 models
    that are traditionally supported (plus the endianness issue), but gcc can
    only generate 2 of those.  The third is APCS_FLOAT, where arguments to
@@ -163,6 +165,7 @@ struct gdbarch_tdep
 				   have_vfp_pseudos.  */
   int have_neon;		/* Do we have a NEON unit?  */
 
+  int is_m;			/* Does the target follow the "M" profile.  */
   CORE_ADDR lowest_pc;		/* Lowest address at which instructions 
 				   will appear.  */
 
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.758
diff -u -p -r1.758 gdb.texinfo
--- doc/gdb.texinfo	23 Aug 2010 20:29:19 -0000	1.758
+++ doc/gdb.texinfo	24 Aug 2010 15:47:06 -0000
@@ -35647,10 +35647,16 @@ registers using the capitalization used 
 @subsection ARM Features
 @cindex target descriptions, ARM features
 
-The @samp{org.gnu.gdb.arm.core} feature is required for ARM targets.
+The @samp{org.gnu.gdb.arm.core} feature is required for non-M-profile
+ARM targets.
 It should contain registers @samp{r0} through @samp{r13}, @samp{sp},
 @samp{lr}, @samp{pc}, and @samp{cpsr}.
 
+For M-profile targets (e.g. Cortex-M3), the @samp{org.gnu.gdb.arm.core}
+feature is replaced by @samp{org.gnu.gdb.arm.m-profile}.  It should contain
+registers @samp{r0} through @samp{r13}, @samp{sp}, @samp{lr}, @samp{pc},
+and @samp{xpsr}.
+
 The @samp{org.gnu.gdb.arm.fpa} feature is optional.  If present, it
 should contain registers @samp{f0} through @samp{f7} and @samp{fps}.
 
Index: features/arm-m-profile.xml
===================================================================
RCS file: features/arm-m-profile.xml
diff -N features/arm-m-profile.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-m-profile.xml	24 Aug 2010 15:47:06 -0000
@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.arm.m-profile">
+  <reg name="r0" bitsize="32"/>
+  <reg name="r1" bitsize="32"/>
+  <reg name="r2" bitsize="32"/>
+  <reg name="r3" bitsize="32"/>
+  <reg name="r4" bitsize="32"/>
+  <reg name="r5" bitsize="32"/>
+  <reg name="r6" bitsize="32"/>
+  <reg name="r7" bitsize="32"/>
+  <reg name="r8" bitsize="32"/>
+  <reg name="r9" bitsize="32"/>
+  <reg name="r10" bitsize="32"/>
+  <reg name="r11" bitsize="32"/>
+  <reg name="r12" bitsize="32"/>
+  <reg name="sp" bitsize="32" type="data_ptr"/>
+  <reg name="lr" bitsize="32"/>
+  <reg name="pc" bitsize="32" type="code_ptr"/>
+  <reg name="xpsr" bitsize="32" regnum="25"/>
+</feature>
Index: features/arm-with-m.c
===================================================================
RCS file: features/arm-with-m.c
diff -N features/arm-with-m.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m.c	24 Aug 2010 15:47:06 -0000
@@ -0,0 +1,35 @@
+/* THIS FILE IS GENERATED.  Original: arm-with-m.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arm_with_m;
+static void
+initialize_tdesc_arm_with_m (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+  struct tdesc_type *field_type, *type;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "sp", 13, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "lr", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "pc", 15, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
+
+  tdesc_arm_with_m = result;
+}
Index: features/arm-with-m.xml
===================================================================
RCS file: features/arm-with-m.xml
diff -N features/arm-with-m.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m.xml	24 Aug 2010 15:47:06 -0000
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <xi:include href="arm-m-profile.xml"/>
+</target>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-08-16 18:06       ` Daniel Jacobowitz
  2010-08-17  9:00         ` Matthew Gretton-Dann
@ 2010-10-29 23:47         ` Jonathan Larmour
  2010-11-01  3:40           ` Daniel Jacobowitz
  1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Larmour @ 2010-10-29 23:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Richard Earnshaw, Kazu Hirata

On 16/08/10 19:05, Daniel Jacobowitz wrote:
> On Thu, Jun 24, 2010 at 09:03:52PM +0100, Richard Earnshaw wrote:
>> I'd suggest
>>
>> org.gnu.gdb.arm.m-profile
> 
> Here's a version addressing comments from Richard, Matthew, and Doug,
> and also incorporating Jonathan Larmour's changes.
> 
> I've moved the is_m check lower in arm_pc_is_thumb, to make "set arm
> force-mode" continue to do something sensible; you could arguably push
> it further down, but this is at least an improvement.
> 
> Tested on ARM EABI and by hand; any comments?

Sorry that it's taken me until now to try out this patch. I also see it's
checked in to CVS which is fair enough.

> Jonathan, how's this
> work for you?  If your target does not give GDB any registers, then
> this ought to just switch an M-profile binary over to the xpsr.  If
> your debug stub previously sends a bogus "cpsr", then it's hard to say
> what will happen.

I'm not quite sure what you mean by this since the xpsr and cpsr are given
the same register number so I thought that what happens at the remote
protocol level would be the same?

> I wouldn't be surprised at an error - any target
> which does that, probably also supplies the FPA registers.

Indeed it does. And that makes GDB unhappy, claiming the response to the
$g packet is too long.

>  If it's
> really necessary, we can try to auto-detect that case in the remote
> protocol, but really this is what the XML register descriptions are
> for.

It does create a problem for me at least because we have existing GDB
stubs implementations programmed into flash on Cortex-M targets (it's hard
to do anything else but program into flash with these small chips). So CVS
GDB (and presumably GDB 7.3+) will no longer work with these targets. If
we change the register layout to avoid the FPA registers in the future,
then GDB <= 7.2 won't work, although for that scenario there is at least
the possibility of implementing the XML.

But I can't do anything about the existing stubs out there, other than get
users to reflash their targets which isn't great, or just not use GDB
>7.2. The existing GDB debug stubs don't return any XML target
description, so we can't go back and add that obviously. So there is a
backward compatibility issue.

I also wonder about any JTAG debuggers which support Cortex-M (e.g.
Ronetix PEEDI). Do they return XML data? I haven't actually tried it with
Cortex-M, but looking at a GDB protocol trace with the PEEDI on a
different ARM target, no XML info is provided. If that's also true for
Cortex-M (which seems likely) then these JTAG debuggers will also stop
working. That would probably adversely affect a lot more people than just
the developers using the eCos GDB stubs.

So I think an alternative may be needed. Either the auto-detection you
refer to (although I smell layering violation), or perhaps a 'set' command.

Jifl

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-10-29 23:47         ` Jonathan Larmour
@ 2010-11-01  3:40           ` Daniel Jacobowitz
  2010-11-03  3:21             ` Jonathan Larmour
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Jacobowitz @ 2010-11-01  3:40 UTC (permalink / raw)
  To: Jonathan Larmour; +Cc: gdb-patches, Richard Earnshaw, Kazu Hirata

On Sat, Oct 30, 2010 at 12:46:53AM +0100, Jonathan Larmour wrote:
> > Jonathan, how's this
> > work for you?  If your target does not give GDB any registers, then
> > this ought to just switch an M-profile binary over to the xpsr.  If
> > your debug stub previously sends a bogus "cpsr", then it's hard to say
> > what will happen.
> 
> I'm not quite sure what you mean by this since the xpsr and cpsr are given
> the same register number so I thought that what happens at the remote
> protocol level would be the same?

The T bit is in a different place.  There's at least one existing stub
which fakes a "CPSR" register when debugging Cortex-M devices with an
XPSR, and moves the T bit to a different place.

Are your stubs returning FPA registers and a fake CPSR, or are they
returning FPA registers and an XPSR?  Both are pretty wonky.

> >  If it's
> > really necessary, we can try to auto-detect that case in the remote
> > protocol, but really this is what the XML register descriptions are
> > for.
> 
> It does create a problem for me at least because we have existing GDB
> stubs implementations programmed into flash on Cortex-M targets (it's hard
> to do anything else but program into flash with these small chips). So CVS
> GDB (and presumably GDB 7.3+) will no longer work with these
> targets.

Sure it will.  You can override this by providing an appropriate XML
file to GDB via set tdesc filename (barring discussion, above, of the
T bit).  One which matches your stub.

Or, you can implement the auto-detection somehow; GDB has
infrastructure for that in general, but this case is a bit peculiar.
I don't know enough about what your stub returns to say for sure.

> I also wonder about any JTAG debuggers which support Cortex-M (e.g.
> Ronetix PEEDI).

I do not have access to any debugger which supports Cortex-M and the
GDB protocol but not XML, sorry.  Someone who does needs to figure out
what they do.

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-11-01  3:40           ` Daniel Jacobowitz
@ 2010-11-03  3:21             ` Jonathan Larmour
  2010-11-03 13:17               ` Daniel Jacobowitz
  0 siblings, 1 reply; 14+ messages in thread
From: Jonathan Larmour @ 2010-11-03  3:21 UTC (permalink / raw)
  To: gdb-patches

On 01/11/10 03:39, Daniel Jacobowitz wrote:
> On Sat, Oct 30, 2010 at 12:46:53AM +0100, Jonathan Larmour wrote:
>> I'm not quite sure what you mean by this since the xpsr and cpsr are given
>> the same register number so I thought that what happens at the remote
>> protocol level would be the same?
> 
> The T bit is in a different place.  There's at least one existing stub
> which fakes a "CPSR" register when debugging Cortex-M devices with an
> XPSR, and moves the T bit to a different place.

Oh, I see. Presumably to work around the absence of this patch!

> Are your stubs returning FPA registers and a fake CPSR, or are they
> returning FPA registers and an XPSR?  Both are pretty wonky.

FPA registers and an XPSR. Because up to this patch (so including GDB 7.2)
the FPA registers were expected by GDB by default. We and others just
complied with GDB's requirements.

>>>  If it's
>>> really necessary, we can try to auto-detect that case in the remote
>>> protocol, but really this is what the XML register descriptions are
>>> for.
>>
>> It does create a problem for me at least because we have existing GDB
>> stubs implementations programmed into flash on Cortex-M targets (it's hard
>> to do anything else but program into flash with these small chips). So CVS
>> GDB (and presumably GDB 7.3+) will no longer work with these
>> targets.
> 
> Sure it will.

Well, it won't work "out of the box".

> You can override this by providing an appropriate XML
> file to GDB via set tdesc filename (barring discussion, above, of the
> T bit).  One which matches your stub.

Alas, this isn't so easy for GUI users, including Eclipse users.

> Or, you can implement the auto-detection somehow; GDB has
> infrastructure for that in general, but this case is a bit peculiar.
> I don't know enough about what your stub returns to say for sure.

Ah, I thought you had already got an idea for how auto-detection would work.

>> I also wonder about any JTAG debuggers which support Cortex-M (e.g.
>> Ronetix PEEDI).
> 
> I do not have access to any debugger which supports Cortex-M and the
> GDB protocol but not XML, sorry.  Someone who does needs to figure out
> what they do.

I was able to try a PEEDI on a Cortex-M board today. It definitely does
not support XML-based target descriptions in general. I'd be amazed if it
was alone. I can confirm that everything appears ok if I do provide an
appropriate target description using "set tdesc ...".

I do understand that Cortex-M (or anything marked as EABI) doesn't have
FPA registers, but nor have most ARM cores for a long time. There are
quite a number of JTAG models out there which claim to support Cortex-M
with GDB. Given what this patch adds, none of them would support returning
the correct target description (it would have to reference
"org.gnu.gdb.arm.m-profile" so that gdbarch->is_m gets set, but that
feature is only added in this patch).

I think there's a more general issue here about whether it's expected to
maintain support for existing working stub implementations such that they
can be used as they have been used even when GDB is upgraded. Changing the
interface with GDB remote stubs used to be a big no-no for GDB. But is it
now the case with the target description support that it's considered
acceptable to break compatibility with _existing_ stubs as long as a
target description file can fix it back up again? I think some users would
struggle working out what the cause of failures like "Remote 'g' packet
reply is too long" is, nevermind how to fix this after starting to use a
newer GDB.

If this is the policy then that's fine, as long as people understand that.
If it's not, then the FPA regs (even though unused in EABI) perhaps ought
to go back in. No matter what I can deal with my own GDB stub whatever is
decided; I'm a bit more concerned about the JTAG debuggers. I'm fully
expecting you to tell me that this is the policy now and the JTAG
manufacturers will have to work out how to update customers in the field,
but I'd like to know for definite.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-11-03  3:21             ` Jonathan Larmour
@ 2010-11-03 13:17               ` Daniel Jacobowitz
  2010-11-10  2:21                 ` Jonathan Larmour
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Jacobowitz @ 2010-11-03 13:17 UTC (permalink / raw)
  To: Jonathan Larmour; +Cc: gdb-patches

On Wed, Nov 03, 2010 at 03:21:24AM +0000, Jonathan Larmour wrote:
> I think there's a more general issue here about whether it's expected to
> maintain support for existing working stub implementations such that they
> can be used as they have been used even when GDB is upgraded. Changing the
> interface with GDB remote stubs used to be a big no-no for GDB.

In general, I want to keep existing stubs working.  But someone who
has access to an affected stub needs to lend a hand here; I don't and
can't test anything.

With stubs out there sending both FPA-regs+fake-CPSR and
FPA-regs+XPSR, I don't know how to make it work; in fact, I don't know
how FPA-regs+XPSR ever worked.

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Add support for ARMv7M devices.
  2010-11-03 13:17               ` Daniel Jacobowitz
@ 2010-11-10  2:21                 ` Jonathan Larmour
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Larmour @ 2010-11-10  2:21 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

On 03/11/10 13:17, Daniel Jacobowitz wrote:
> On Wed, Nov 03, 2010 at 03:21:24AM +0000, Jonathan Larmour wrote:
>> I think there's a more general issue here about whether it's expected to
>> maintain support for existing working stub implementations such that they
>> can be used as they have been used even when GDB is upgraded. Changing the
>> interface with GDB remote stubs used to be a big no-no for GDB.
> 
> In general, I want to keep existing stubs working.  But someone who
> has access to an affected stub needs to lend a hand here; I don't and
> can't test anything.

I believe I've now worked out a solution, and it's not that complicated in
fact. See the attached patch. The key seems to be to let remote.c choose
the tdesc.

2010-11-09  Jonathan Larmour  <jifl@eCosCentric.com>

	* arm-tdep.c (arm_gdbarch_init): Allow M-profile targets' remote
	stubs to provide 'g' packets with or without FPA registers.

> With stubs out there sending both FPA-regs+fake-CPSR and
> FPA-regs+XPSR, I don't know how to make it work; in fact, I don't know
> how FPA-regs+XPSR ever worked.

It's clearly impossible to tell the difference between fake-CPSR and XPSR.
But even an XML target description can't make that case work right, so I
don't think that's too bad for us not to cope with. Unless you have some
ideas (a 'set' option seems overkill).

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

[-- Attachment #2: mprofile-backcompat.patch --]
[-- Type: text/plain, Size: 5878 bytes --]

diff -x CVS -x .svn -x '*~' -x '.#*' -x autom4te.cache -urpN src.old/gdb/arm-tdep.c src/gdb/arm-tdep.c
--- src.old/gdb/arm-tdep.c	2010-11-10 01:25:08.000000000 +0000
+++ src/gdb/arm-tdep.c	2010-11-10 02:00:31.000000000 +0000
@@ -42,6 +42,7 @@
 #include "prologue-value.h"
 #include "target-descriptions.h"
 #include "user-regs.h"
+#include "remote.h"
 
 #include "arm-tdep.h"
 #include "gdb/sim-arm.h"
@@ -54,6 +55,7 @@
 #include "vec.h"
 
 #include "features/arm-with-m.c"
+#include "features/arm-with-m-fpa.c"
 
 static int arm_debug;
 
@@ -7130,7 +7132,7 @@ arm_gdbarch_init (struct gdbarch_info in
 		  && (attr_arch == TAG_CPU_ARCH_V6_M
 		      || attr_arch == TAG_CPU_ARCH_V6S_M
 		      || attr_profile == 'M'))
-		tdesc = tdesc_arm_with_m;
+		is_m = 1;
 #endif
 	    }
 
@@ -7192,6 +7194,8 @@ arm_gdbarch_init (struct gdbarch_info in
 	  else
 	    is_m = 1;
 	}
+      else
+        is_m = 0;
 
       tdesc_data = tdesc_data_alloc ();
 
@@ -7566,6 +7570,29 @@ arm_gdbarch_init (struct gdbarch_info in
     user_reg_add (gdbarch, arm_register_aliases[i].name,
 		  value_of_arm_user_reg, &arm_register_aliases[i].regnum);
 
+  if (is_m)
+    {
+      /* For backward-compatibility we allow two 'g' packet lengths with
+         the remote protocol depending on whether FPA registers are
+         supplied. M-profile targets do not have FPA registers, but some
+         stubs already exist in the wild which use a 'g' packet which
+         supplies them albeit with dummy values. The packet format which
+         includes FPA registers should be considered deprecated for
+         M-profile targets.
+
+         The register sizes are fixed for these target descriptions in the
+         XML files, so we can hard-code them: r0-12,sp,lr,pc,xpsr for
+         tdesc_arm_with_m; and additionally f0-f7 and fps for
+         tdesc_arm_with_m_fpa.  */
+      
+      register_remote_g_packet_guess (gdbarch,
+                                      17 * INT_REGISTER_SIZE,
+                                      tdesc_arm_with_m);
+      register_remote_g_packet_guess (gdbarch,
+                                      18 * INT_REGISTER_SIZE +
+                                      8 * FP_REGISTER_SIZE,
+                                      tdesc_arm_with_m_fpa);
+    }
   return gdbarch;
 }
 
@@ -7609,6 +7636,7 @@ _initialize_arm_tdep (void)
 
   /* Initialize the standard target descriptions.  */
   initialize_tdesc_arm_with_m ();
+  initialize_tdesc_arm_with_m_fpa ();
 
   /* Get the number of possible sets of register names defined in opcodes.  */
   num_disassembly_options = get_arm_regname_num_options ();
diff -x CVS -x .svn -x '*~' -x '.#*' -x autom4te.cache -urpN src.old/gdb/features/arm-with-m-fpa.c src/gdb/features/arm-with-m-fpa.c
--- src.old/gdb/features/arm-with-m-fpa.c	1970-01-01 01:00:00.000000000 +0100
+++ src/gdb/features/arm-with-m-fpa.c	2010-11-10 02:00:31.000000000 +0000
@@ -0,0 +1,46 @@
+/* THIS FILE IS GENERATED.  Original: arm-with-m-fpa.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arm_with_m_fpa;
+static void
+initialize_tdesc_arm_with_m_fpa (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+  struct tdesc_type *field_type, *type;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "sp", 13, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "lr", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "pc", 15, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.fpa");
+  tdesc_create_reg (feature, "f0", 16, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f1", 17, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f2", 18, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f3", 19, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f4", 20, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f5", 21, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f6", 22, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "f7", 23, 1, NULL, 96, "arm_fpa_ext");
+  tdesc_create_reg (feature, "fps", 24, 1, NULL, 32, "int");
+
+  tdesc_arm_with_m_fpa = result;
+}
diff -x CVS -x .svn -x '*~' -x '.#*' -x autom4te.cache -urpN src.old/gdb/features/arm-with-m-fpa.xml src/gdb/features/arm-with-m-fpa.xml
--- src.old/gdb/features/arm-with-m-fpa.xml	1970-01-01 01:00:00.000000000 +0100
+++ src/gdb/features/arm-with-m-fpa.xml	2010-11-10 02:00:31.000000000 +0000
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <xi:include href="arm-m-profile.xml"/>
+  <xi:include href="arm-fpa.xml"/>
+</target>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-11-10  2:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-09 14:03 [patch] Add support for ARMv7M devices Kazu Hirata
2010-06-09 22:29 ` Doug Evans
2010-06-09 23:04 ` Richard Earnshaw
2010-06-24 16:42   ` Daniel Jacobowitz
2010-06-24 20:04     ` Richard Earnshaw
2010-08-16 18:06       ` Daniel Jacobowitz
2010-08-17  9:00         ` Matthew Gretton-Dann
2010-08-24 15:56           ` Daniel Jacobowitz
2010-10-29 23:47         ` Jonathan Larmour
2010-11-01  3:40           ` Daniel Jacobowitz
2010-11-03  3:21             ` Jonathan Larmour
2010-11-03 13:17               ` Daniel Jacobowitz
2010-11-10  2:21                 ` Jonathan Larmour
2010-06-10  8:56 ` Matthew Gretton-Dann

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