[PATCH] MIPS: Handle the DSP registers for bare metal

[PATCH] MIPS: Handle the DSP registers for bare metal

[Yao Qi]

In 2007, Maciej submitted the patch handling DSP registers on both
linux and bare metal targets.

  MIPS: Handle the DSP registers
  https://sourceware.org/ml/gdb-patches/2007-12/msg00150.html

the patch was reviewed but didn't go in.  Then Maciej resubmit the
patch again only for the linux target, which was committed.

  [PATCH] MIPS/Linux: DSP ASE support
  https://sourceware.org/ml/gdb-patches/2011-11/msg00586.html

This patch is about the left over of handling DSP registers.  Since
the offset and layout of DSP registers vary between and linux and bare
metal, this patch is to adjust the offset for these registers.

gdb:

2014-12-18  Maciej W. Rozycki  <macro@codesourcery.com>
	    Chris Dearman  <chris@mips.com>

	* mips-tdep.c (NUM_MIPS_PROCESSOR_REGS): Set from
	MIPS_LAST_EMBED_REGNUM.
	(mips_gdbarch_init): Add embedded DSP support.
	* mips-tdep.h (MIPS_EMBED_CP2_REGNUM): Offset to CP2 registers.
	(MIPS_EMBED_DSPACC_REGNUM): Offset to DSP accumulator registers.
	(MIPS_EMBED_DSPCTL_REGNUM): Offset to DSP control registers.
	(MIPS_LAST_EMBED_REGNUM): Update accordingly.
	(MIPS_EMBED_NUM_REGS): New value to make sure that an even
	number of registers is used.
---
 gdb/mips-tdep.c | 15 ++++++++++-----
 gdb/mips-tdep.h | 23 ++++++++++++++++++++---
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 60f43ac..1fdb216 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -546,7 +546,7 @@ static struct cmd_list_element *showmipscmdlist = NULL;
    are listed in the following tables.  */
 
 enum
-{ NUM_MIPS_PROCESSOR_REGS = (90 - 32) };
+{ NUM_MIPS_PROCESSOR_REGS = (MIPS_LAST_EMBED_REGNUM + 1 - 32) };
 
 /* Generic MIPS.  */
 
@@ -8191,7 +8191,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   struct gdbarch_tdep *tdep;
   int elf_flags;
   enum mips_abi mips_abi, found_abi, wanted_abi;
-  int i, num_regs;
+  int i, num_regs, dsp_space;
   enum mips_fpu_type fpu_type;
   struct tdesc_arch_data *tdesc_data = NULL;
   int elf_fpu_type = Val_GNU_MIPS_ABI_FP_ANY;
@@ -8214,6 +8214,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
       mips_regnum.fp_implementation_revision = 70;
       mips_regnum.dspacc = dspacc = -1;
       mips_regnum.dspctl = dspctl = -1;
+      dsp_space = 0;
       num_regs = 71;
       reg_names = mips_irix_reg_names;
     }
@@ -8231,6 +8232,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
       mips_regnum.dspctl = -1;
       dspacc = 72;
       dspctl = 78;
+      dsp_space = 0;
       num_regs = 79;
       reg_names = mips_linux_reg_names;
     }
@@ -8244,8 +8246,9 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
       mips_regnum.fp0 = MIPS_EMBED_FP0_REGNUM;
       mips_regnum.fp_control_status = 70;
       mips_regnum.fp_implementation_revision = 71;
-      mips_regnum.dspacc = dspacc = -1;
-      mips_regnum.dspctl = dspctl = -1;
+      mips_regnum.dspacc = dspacc = MIPS_EMBED_DSPACC_REGNUM;
+      mips_regnum.dspctl = dspctl = MIPS_EMBED_DSPCTL_REGNUM;
+      dsp_space = 1;
       num_regs = MIPS_LAST_EMBED_REGNUM + 1;
       if (info.bfd_arch_info != NULL
           && info.bfd_arch_info->mach == bfd_mach_mips3900)
@@ -8357,16 +8360,18 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	  /* The DSP registers are optional; it's OK if they are absent.  */
 	  if (feature != NULL)
 	    {
-	      i = 0;
+	      i = dsp_space;
 	      valid_p = 1;
 	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
 						  dspacc + i++, "hi1");
 	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
 						  dspacc + i++, "lo1");
+	      i += dsp_space;
 	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
 						  dspacc + i++, "hi2");
 	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
 						  dspacc + i++, "lo2");
+	      i += dsp_space;
 	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
 						  dspacc + i++, "hi3");
 	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
diff --git a/gdb/mips-tdep.h b/gdb/mips-tdep.h
index 186f158..f781245 100644
--- a/gdb/mips-tdep.h
+++ b/gdb/mips-tdep.h
@@ -139,9 +139,26 @@ enum
   MIPS_EMBED_PC_REGNUM = 37,
   MIPS_EMBED_FP0_REGNUM = 38,
   MIPS_UNUSED_REGNUM = 73,	/* Never used, FIXME.  */
-  MIPS_FIRST_EMBED_REGNUM = 74,	/* First CP0 register for embedded use.  */
-  MIPS_PRID_REGNUM = 89,	/* Processor ID.  */
-  MIPS_LAST_EMBED_REGNUM = 89	/* Last one.  */
+  MIPS_FIRST_EMBED_REGNUM = 74,	/* First CP register for embedded use.  */
+  MIPS_EMBED_CP0_REGNUM = MIPS_FIRST_EMBED_REGNUM,
+				/* CP0 data registers: 8 banks of 32.  */
+  MIPS_PRID_REGNUM = (MIPS_EMBED_CP0_REGNUM + 15),
+				/* Processor ID.  */
+  MIPS_EMBED_CP2_REGNUM = (MIPS_EMBED_CP0_REGNUM + 8 * 32),
+				/* CP2 data registers: 8 banks of 32.  */
+  MIPS_EMBED_CP2CTL_REGNUM = (MIPS_EMBED_CP2_REGNUM + 8 * 32),
+				/* CP2 control registers: 32.  */
+  MIPS_EMBED_DSPACC_REGNUM = (MIPS_EMBED_CP2CTL_REGNUM + 32),
+				/* DSP/SmartMIPS registers:
+				   ACX, Hi1, Lo1, ACX1,
+				   Hi2, Lo2, ACX2, Hi3, Lo3, ACX3.  */
+  MIPS_EMBED_DSPCTL_REGNUM = (MIPS_EMBED_DSPACC_REGNUM + 10),
+				/* DSP DSPCTL0..1 registers.  */
+  MIPS_EMBED_NUM_REGS = (MIPS_EMBED_DSPCTL_REGNUM + 2),
+				/* Total number of actual registers.  */
+  MIPS_LAST_EMBED_REGNUM
+    = ((MIPS_EMBED_NUM_REGS + MIPS_EMBED_NUM_REGS % 2) - 1)
+				/* Last one, including padding to even.  */
 };
 
 /* Defined in mips-tdep.c and used in remote-mips.c.  */
-- 
1.9.3

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Pedro Alves]

On 12/18/2014 01:25 PM, Yao Qi wrote:

> 
> gdb:
> 
> 2014-12-18  Maciej W. Rozycki  <macro@codesourcery.com>
> 	    Chris Dearman  <chris@mips.com>
> 
> 	* mips-tdep.c (NUM_MIPS_PROCESSOR_REGS): Set from
> 	MIPS_LAST_EMBED_REGNUM.
> 	(mips_gdbarch_init): Add embedded DSP support.
> 	* mips-tdep.h (MIPS_EMBED_CP2_REGNUM): Offset to CP2 registers.
> 	(MIPS_EMBED_DSPACC_REGNUM): Offset to DSP accumulator registers.
> 	(MIPS_EMBED_DSPCTL_REGNUM): Offset to DSP control registers.
> 	(MIPS_LAST_EMBED_REGNUM): Update accordingly.
> 	(MIPS_EMBED_NUM_REGS): New value to make sure that an even
> 	number of registers is used.
> ---
>  gdb/mips-tdep.c | 15 ++++++++++-----
>  gdb/mips-tdep.h | 23 ++++++++++++++++++++---
>  2 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index 60f43ac..1fdb216 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -546,7 +546,7 @@ static struct cmd_list_element *showmipscmdlist = NULL;
>     are listed in the following tables.  */
>  
>  enum
> -{ NUM_MIPS_PROCESSOR_REGS = (90 - 32) };
> +{ NUM_MIPS_PROCESSOR_REGS = (MIPS_LAST_EMBED_REGNUM + 1 - 32) };
>  
>  /* Generic MIPS.  */
>  
> @@ -8191,7 +8191,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    struct gdbarch_tdep *tdep;
>    int elf_flags;
>    enum mips_abi mips_abi, found_abi, wanted_abi;
> -  int i, num_regs;
> +  int i, num_regs, dsp_space;
>    enum mips_fpu_type fpu_type;
>    struct tdesc_arch_data *tdesc_data = NULL;
>    int elf_fpu_type = Val_GNU_MIPS_ABI_FP_ANY;
> @@ -8214,6 +8214,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>        mips_regnum.fp_implementation_revision = 70;
>        mips_regnum.dspacc = dspacc = -1;
>        mips_regnum.dspctl = dspctl = -1;
> +      dsp_space = 0;
>        num_regs = 71;
>        reg_names = mips_irix_reg_names;
>      }
> @@ -8231,6 +8232,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>        mips_regnum.dspctl = -1;
>        dspacc = 72;
>        dspctl = 78;
> +      dsp_space = 0;
>        num_regs = 79;
>        reg_names = mips_linux_reg_names;
>      }
> @@ -8244,8 +8246,9 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>        mips_regnum.fp0 = MIPS_EMBED_FP0_REGNUM;
>        mips_regnum.fp_control_status = 70;
>        mips_regnum.fp_implementation_revision = 71;
> -      mips_regnum.dspacc = dspacc = -1;
> -      mips_regnum.dspctl = dspctl = -1;
> +      mips_regnum.dspacc = dspacc = MIPS_EMBED_DSPACC_REGNUM;
> +      mips_regnum.dspctl = dspctl = MIPS_EMBED_DSPCTL_REGNUM;
> +      dsp_space = 1;
>        num_regs = MIPS_LAST_EMBED_REGNUM + 1;
>        if (info.bfd_arch_info != NULL
>            && info.bfd_arch_info->mach == bfd_mach_mips3900)
> @@ -8357,16 +8360,18 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  	  /* The DSP registers are optional; it's OK if they are absent.  */
>  	  if (feature != NULL)
>  	    {
> -	      i = 0;
> +	      i = dsp_space;
>  	      valid_p = 1;
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "hi1");
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "lo1");
> +	      i += dsp_space;
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "hi2");
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "lo2");
> +	      i += dsp_space;
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "hi3");
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,

Took me a bit to grok this, but this is adding slack for ACXn, right?
But it seems like nothing in GDB knows about those ACX registers.  I
guess I'm being dense, but why is this patch needed then?  They should still
be accessible to the user even without this change, right?  Assuming the
description is including them.

Thanks,
Pedro Alves

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Took me a bit to grok this, but this is adding slack for ACXn, right?

Sorry, what do you mean by "slack" here?  Is it "gap" or something else?
The offsets of DSP registers are different on linux and bare metal, so
this patch gives the correct offset or layout to them.

> But it seems like nothing in GDB knows about those ACX registers.  I
> guess I'm being dense, but why is this patch needed then?  They should still
> be accessible to the user even without this change, right?  Assuming the
> description is including them.

We want the number of these registers are fixed, and these fixed numbers
will be used in a follow-up patch about dynamic registers discovery
(which is about reading available config registers and parsing bits in them)
MIPS architecture defines 50+ subset of optional CP0 registers, so the
number of variants is too high to make current static register
description approach useless.

-- 
Yao (齐尧)

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Pedro Alves]

On 12/19/2014 03:54 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Took me a bit to grok this, but this is adding slack for ACXn, right?
> 
> Sorry, what do you mean by "slack" here?  Is it "gap" or something else?

Yes, "gap".

> The offsets of DSP registers are different on linux and bare metal, so
> this patch gives the correct offset or layout to them.

The proper solution for this issue is to decouple GDB's internal
register numbers from the target's g/G packet layout, which is exactly
what happens when you have a description -- GDB uses the offsets found
in the target description.  And you're touching code that is parsing a
description, so the real issue should be in the target description.

> 
>> But it seems like nothing in GDB knows about those ACX registers.  I
>> guess I'm being dense, but why is this patch needed then?  They should still
>> be accessible to the user even without this change, right?  Assuming the
>> description is including them.
> 
> We want the number of these registers are fixed, and these fixed numbers
> will be used in a follow-up patch about dynamic registers discovery
> (which is about reading available config registers and parsing bits in them)
> MIPS architecture defines 50+ subset of optional CP0 registers, so the
> number of variants is too high to make current static register
> description approach useless.

I think this should be discussed further.

Thanks,
Pedro Alves

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> The proper solution for this issue is to decouple GDB's internal
> register numbers from the target's g/G packet layout, which is exactly
> what happens when you have a description -- GDB uses the offsets found
> in the target description.  And you're touching code that is parsing a
> description, so the real issue should be in the target description.

So, these dsp registers on bare metal target have different layout from
them on linux target, the stub should send the org.gnu.gdb.mips.dsp
feature with some different "regnum", is that correct?  On linux target,
gdbserver sends

<feature name="org.gnu.gdb.mips.dsp">
  <reg name="hi1" bitsize="32" regnum="72"/>
  ...
</feature>

on bare metal target, the stub should send

<feature name="org.gnu.gdb.mips.dsp">
  <reg name="hi1" bitsize="32" regnum="73"/>
  ...
</feature>

is it right?

-- 
Yao (齐尧)

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Pedro Alves]

On 12/19/2014 01:22 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> The proper solution for this issue is to decouple GDB's internal
>> register numbers from the target's g/G packet layout, which is exactly
>> what happens when you have a description -- GDB uses the offsets found
>> in the target description.  And you're touching code that is parsing a
>> description, so the real issue should be in the target description.
> 
> So, these dsp registers on bare metal target have different layout from
> them on linux target, the stub should send the org.gnu.gdb.mips.dsp
> feature with some different "regnum", is that correct?  On linux target,
> gdbserver sends
> 
> <feature name="org.gnu.gdb.mips.dsp">
>   <reg name="hi1" bitsize="32" regnum="72"/>
>   ...
> </feature>
> 
> on bare metal target, the stub should send
> 
> <feature name="org.gnu.gdb.mips.dsp">
>   <reg name="hi1" bitsize="32" regnum="73"/>
>   ...
> </feature>
> 
> is it right?
> 

That should work.  "regnum=" is really only needed to force gaps.  If
you include extra registers in the description, GDB will display them.
So you could also do:

<feature name="org.gnu.gdb.mips.dsp">
  <reg name="acx" bitsize="32"/>
  <reg name="hi1" bitsize="32"/>
  <reg name="lo1" bitsize="32"/>
  <reg name="acx1" bitsize="32"/>
  <reg name="hi2" bitsize="32"/>
  <reg name="lo2" bitsize="32"/>
  <reg name="acx2" bitsize="32"/>
  <reg name="hi3" bitsize="32"/>
  <reg name="lo3" bitsize="32"/>
  <reg name="acx2" bitsize="32"/>
  <reg name="dspctl" bitsize="32"/>
</feature>

Thanks,
Pedro Alves

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Maciej W. Rozycki]

On Fri, 19 Dec 2014, Pedro Alves wrote:

> >> Took me a bit to grok this, but this is adding slack for ACXn, right?
> > 
> > Sorry, what do you mean by "slack" here?  Is it "gap" or something else?
> 
> Yes, "gap".
> 
> > The offsets of DSP registers are different on linux and bare metal, so
> > this patch gives the correct offset or layout to them.
> 
> The proper solution for this issue is to decouple GDB's internal
> register numbers from the target's g/G packet layout, which is exactly
> what happens when you have a description -- GDB uses the offsets found
> in the target description.  And you're touching code that is parsing a
> description, so the real issue should be in the target description.

 I'm not sure offhand whether the piece of patch proposed you refer to 
here is correct or not, but the overall scope of this and the other patch 
Yao has mentioned yet outstanding is support for legacy bare-metal RSP 
stubs that have no notion of target descriptions and may even predate 
GDB's support for these descriptions, and yet they want to make all 
processor registers available for inspection and modification by GDB.  
This code comes from MIPS UK and dates back to early 2000s and I think it 
would be good having it upstream so that standard GDB can talk to these 
stubs.  The fixed layout of the g/G packet and corresponding p/P packet 
offsets have been set by the bare-metal SDE toolchain years ago.

 The layout has one significant shortcoming in that it does not support 
64-bit FPRs (CP0.Status.FR=1 mode) in the 32-bit mode (o32 ABI or plain 
32-bit processors) as there is no room provided for the high 32-bit parts 
of these registers.

> >> But it seems like nothing in GDB knows about those ACX registers.  I
> >> guess I'm being dense, but why is this patch needed then?  They should still
> >> be accessible to the user even without this change, right?  Assuming the
> >> description is including them.

 The gaps for the extra ACX registers have never been actually filled, no 
processor has ever existed that supported them.  These were provided for 
processors that support the SmartMIPS and the DSP ASE both at a time, but 
no such processor has ever been made after all.  The only place such a 
configuration could potentially be enabled right now in a straightforward 
manner is QEMU, but the emulator does not currently support exchanging the 
extra registers defined by this change (there is some support for poking 
at some of these registers via the `monitor' command though; the usual 
limitations apply, e.g. it can't be used in expressions).

 Some background information: ACX is the ACcumulator eXtension register 
defined by the SmartMIPS ASE and holds the highest part of a number whose 
lower parts are held in the traditional MIPS HI/LO MDU unit's register 
pair aka MDU accumulator (ACX extends the HI/LO register pair).  The DSP 
ASE defines 3 extra HI/LO register pairs for a total of 4 accumulators.  
A processor with both ASEs combined would therefore require 3 extra ACX 
registers (for a total of 4) to extend the 3 additional accumulators.

 Of course the gaps need to be preserved so as to get the offsets of 
registers placed beyond them right, as stubs will include/expect these 
gaps in packets exchanged.

> > We want the number of these registers are fixed, and these fixed numbers
> > will be used in a follow-up patch about dynamic registers discovery
> > (which is about reading available config registers and parsing bits in them)
> > MIPS architecture defines 50+ subset of optional CP0 registers, so the
> > number of variants is too high to make current static register
> > description approach useless.
> 
> I think this should be discussed further.

 Absolutely, having support for target descriptions in bare-metal RSP 
stubs (and any complementing changes possibly required for GDB), will 
certainly be a good feature, lifting the problem with 64-bit FPRs on 
32-bit targets as a side effect too.  However it will not work for older 
bare-metal stubs that have been out there for 10+ years now (and actually 
all in existence right now, I believe).

  Maciej

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Pedro Alves]

On 12/30/2014 01:15 AM, Maciej W. Rozycki wrote:
> On Fri, 19 Dec 2014, Pedro Alves wrote:
> 
>>>> > >> Took me a bit to grok this, but this is adding slack for ACXn, right?
>>> > > 
>>> > > Sorry, what do you mean by "slack" here?  Is it "gap" or something else?
>> > 
>> > Yes, "gap".
>> > 
>>> > > The offsets of DSP registers are different on linux and bare metal, so
>>> > > this patch gives the correct offset or layout to them.
>> > 
>> > The proper solution for this issue is to decouple GDB's internal
>> > register numbers from the target's g/G packet layout, which is exactly
>> > what happens when you have a description -- GDB uses the offsets found
>> > in the target description.  And you're touching code that is parsing a
>> > description, so the real issue should be in the target description.
>  I'm not sure offhand whether the piece of patch proposed you refer to 
> here is correct or not, but the overall scope of this and the other patch 
> Yao has mentioned yet outstanding is support for legacy bare-metal RSP 
> stubs that have no notion of target descriptions and may even predate 
> GDB's support for these descriptions, and yet they want to make all 
> processor registers available for inspection and modification by GDB.  
> This code comes from MIPS UK and dates back to early 2000s and I think it 
> would be good having it upstream so that standard GDB can talk to these 
> stubs.  The fixed layout of the g/G packet and corresponding p/P packet 
> offsets have been set by the bare-metal SDE toolchain years ago.

The way to handle that is still through target descriptions -- if a
target doesn't send a target description, GDB maps known layouts to built-in
target descriptions.  See mips_register_g_packet_guesses.

[snip interesting background info]

Thanks for all that.

-- 
Pedro Alves

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Maciej W. Rozycki]

On Tue, 30 Dec 2014, Pedro Alves wrote:

> >  I'm not sure offhand whether the piece of patch proposed you refer to 
> > here is correct or not, but the overall scope of this and the other patch 
> > Yao has mentioned yet outstanding is support for legacy bare-metal RSP 
> > stubs that have no notion of target descriptions and may even predate 
> > GDB's support for these descriptions, and yet they want to make all 
> > processor registers available for inspection and modification by GDB.  
> > This code comes from MIPS UK and dates back to early 2000s and I think it 
> > would be good having it upstream so that standard GDB can talk to these 
> > stubs.  The fixed layout of the g/G packet and corresponding p/P packet 
> > offsets have been set by the bare-metal SDE toolchain years ago.
> 
> The way to handle that is still through target descriptions -- if a
> target doesn't send a target description, GDB maps known layouts to built-in
> target descriptions.  See mips_register_g_packet_guesses.

 Register probing Yao mentioned is still needed, because in this fixed 
packet format the whole set of architecturally defined register slots is 
exchanged with the RSP stub, e.g. 8 * 32 = 256 CP0 registers, 8 * 32 = 256 
CP2 registers, etc. (as documented by the change to mips-tdep.h proposed), 
even if some are not present, e.g. not all CP0 register slots have already 
been allocated in the architecture so far and the vast majority of them is 
optional.

 The information on which registers are present and which are not will not 
be supplied by the target and has to be determined by gradual discovery, 
that is poking at registers as they are determined to be present, i.e. 
first CP0.PRId that is always there, then CP0.Config0 if present, then 
CP0.Config1 if present, and so on.  Gaps will be present in the packets 
exchanged for the absent registers as with the ACX registers discussed 
previously.

 And if we were to define built-in target descriptions for the possible 
variants of present register sets, then, as noted by Yao, we hit the issue 
of having to make some 2^50 templates (slightly fewer actually, as there 
are some dependencies between the presence of some registers, e.g. you 
can't have a processor that has a CP0.Config2 register, but does not have 
CP0.Config1, the reverse is allowed; similarly a CPU can't have the ACX 
register when it does not have CP0.Config3) which is of course beyond 
technical capabilities as e.g. a 32-bit host will have fewer bytes of 
addressable memory even.

 So some other way has to be invented if you think the current working 
solution is not good enough for some reason.

 NB `mips_register_g_packet_guesses' is not necessarily relevant here as 
there may be registers accessible through the stub via the p/P packets 
only and not included in the g/G packet layout for performance reasons 
(the packet may be shorter than the maximum defined) as to transfer all 
the registers through a JTAG link every time the target is stopped may be 
exceedingly slow, especially when single-stepping.

  Maciej

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Pedro Alves]

On 12/30/2014 12:10 PM, Maciej W. Rozycki wrote:
> On Tue, 30 Dec 2014, Pedro Alves wrote:
> 
>>> > >  I'm not sure offhand whether the piece of patch proposed you refer to 
>>> > > here is correct or not, but the overall scope of this and the other patch 
>>> > > Yao has mentioned yet outstanding is support for legacy bare-metal RSP 
>>> > > stubs that have no notion of target descriptions and may even predate 
>>> > > GDB's support for these descriptions, and yet they want to make all 
>>> > > processor registers available for inspection and modification by GDB.  
>>> > > This code comes from MIPS UK and dates back to early 2000s and I think it 
>>> > > would be good having it upstream so that standard GDB can talk to these 
>>> > > stubs.  The fixed layout of the g/G packet and corresponding p/P packet 
>>> > > offsets have been set by the bare-metal SDE toolchain years ago.
>> > 
>> > The way to handle that is still through target descriptions -- if a
>> > target doesn't send a target description, GDB maps known layouts to built-in
>> > target descriptions.  See mips_register_g_packet_guesses.

>  Register probing Yao mentioned is still needed, because in this fixed 
> packet format the whole set of architecturally defined register slots is 
> exchanged with the RSP stub, e.g. 8 * 32 = 256 CP0 registers, 8 * 32 = 256 
> CP2 registers, etc. (as documented by the change to mips-tdep.h proposed), 
> even if some are not present, e.g. not all CP0 register slots have already 
> been allocated in the architecture so far and the vast majority of them is 
> optional.

That's orthogonal.

> 
>  The information on which registers are present and which are not will not 
> be supplied by the target and has to be determined by gradual discovery, 
> that is poking at registers as they are determined to be present, i.e. 
> first CP0.PRId that is always there, then CP0.Config0 if present, then 
> CP0.Config1 if present, and so on.  Gaps will be present in the packets 
> exchanged for the absent registers as with the ACX registers discussed 
> previously.

> 
>  And if we were to define built-in target descriptions for the possible 
> variants of present register sets, then, as noted by Yao, we hit the issue 
> of having to make some 2^50 templates (slightly fewer actually, as there 

That'd be silly of course, but it's also not what I meant.  The built-in
description is used for the case of a legacy stubs that does not report a
target description, and those stubs you say use a fixed register layout.
So we're talking about _one_ built-in target description that describes
that fixed register layout.

> are some dependencies between the presence of some registers, e.g. you 
> can't have a processor that has a CP0.Config2 register, but does not have 
> CP0.Config1, the reverse is allowed; similarly a CPU can't have the ACX 
> register when it does not have CP0.Config3) which is of course beyond 
> technical capabilities as e.g. a 32-bit host will have fewer bytes of 
> addressable memory even.

>  NB `mips_register_g_packet_guesses' is not necessarily relevant here as 
> there may be registers accessible through the stub via the p/P packets 
> only and not included in the g/G packet layout for performance reasons 

If existing/legacy stubs are doing that in practice, then we can
make gdb pick the fallback description (in case in target doesn't
provide one) depending on selected osabi.

> (the packet may be shorter than the maximum defined) as to transfer all 
> the registers through a JTAG link every time the target is stopped may be 
> exceedingly slow, especially when single-stepping.

For the single-stepping case, the fix should be to include
all the registers GDB needs to compute whether the step finished
directly in the expedite stop reply, though.

Thanks,
Pedro Alves

Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Maciej W. Rozycki]

On Mon, 5 Jan 2015, Pedro Alves wrote:

> >  Register probing Yao mentioned is still needed, because in this fixed 
> > packet format the whole set of architecturally defined register slots is 
> > exchanged with the RSP stub, e.g. 8 * 32 = 256 CP0 registers, 8 * 32 = 256 
> > CP2 registers, etc. (as documented by the change to mips-tdep.h proposed), 
> > even if some are not present, e.g. not all CP0 register slots have already 
> > been allocated in the architecture so far and the vast majority of them is 
> > optional.
> 
> That's orthogonal.
> 
> > 
> >  The information on which registers are present and which are not will not 
> > be supplied by the target and has to be determined by gradual discovery, 
> > that is poking at registers as they are determined to be present, i.e. 
> > first CP0.PRId that is always there, then CP0.Config0 if present, then 
> > CP0.Config1 if present, and so on.  Gaps will be present in the packets 
> > exchanged for the absent registers as with the ACX registers discussed 
> > previously.
> 
> > 
> >  And if we were to define built-in target descriptions for the possible 
> > variants of present register sets, then, as noted by Yao, we hit the issue 
> > of having to make some 2^50 templates (slightly fewer actually, as there 
> 
> That'd be silly of course, but it's also not what I meant.  The built-in
> description is used for the case of a legacy stubs that does not report a
> target description, and those stubs you say use a fixed register layout.
> So we're talking about _one_ built-in target description that describes
> that fixed register layout.

 Addressing the two parts quoted -- do you mean to have a built-in target 
description that optional registers can be further excluded from if not 
actually present, analogously to what the patches proposed do without 
creating this artificial target description?

 Yao, I think it would make sense if you posted the other change I've been 
referring to for a reference, so that Pedro and other people have a chance 
to know what I am talking about.

> > are some dependencies between the presence of some registers, e.g. you 
> > can't have a processor that has a CP0.Config2 register, but does not have 
> > CP0.Config1, the reverse is allowed; similarly a CPU can't have the ACX 
> > register when it does not have CP0.Config3) which is of course beyond 
> > technical capabilities as e.g. a 32-bit host will have fewer bytes of 
> > addressable memory even.
> 
> >  NB `mips_register_g_packet_guesses' is not necessarily relevant here as 
> > there may be registers accessible through the stub via the p/P packets 
> > only and not included in the g/G packet layout for performance reasons 
> 
> If existing/legacy stubs are doing that in practice, then we can
> make gdb pick the fallback description (in case in target doesn't
> provide one) depending on selected osabi.

 They do.  We might be able to make an educated guess from a `g' packet 
anyway, it's not like there is much choice here.  Using the osabi has the 
corner-case drawback of forcing the user to set it manually where no 
executable image of code to debug is available (e.g. debugging ROM 
firmware).  Besides, I believe the osabi is tangential to the debug stub 
being used.

> > (the packet may be shorter than the maximum defined) as to transfer all 
> > the registers through a JTAG link every time the target is stopped may be 
> > exceedingly slow, especially when single-stepping.
> 
> For the single-stepping case, the fix should be to include
> all the registers GDB needs to compute whether the step finished
> directly in the expedite stop reply, though.

 Registers have more recently been already included in the expedite stop 
reply, IIRC they include $pc, $sp, $fp and $s1 (the latter being the 
MIPS16 frame pointer).  That may not cover all the corner cases though, 
e.g. $s2 is needed if single-stepping stops in some MIPS16 thunks, that 
are not worth the performance regression sending extra registers in the 
expedite reply would cause for the general case.

 Finally, in most cases, such as accessing program variables whose storage 
has been optimised to registers, it's the GPR and maybe the FPR register 
set that is ever used.  So the contents of the `g'/`G' packets are really 
a compromise -- performance when accessing all registers is traded for 
performance when accessing the most frequently used ones.

 This is because the `p'/`P' packets have a higher overhead as they 
transfer individual registers, requiring a JTAG probe to do the whole 
dance for the target to switch modes for a single register access.  
Whereas the `g'/`G' packets let a JTAG probe do a bulk transfer and 
therefore do the dance with the target only once.

 By avoiding including infrequently needed registers we're talking of up 
to dozens of seconds to save here per single `g'/`G' packet here.  But 
then a command like:

(gdb) info registers

can take these dozens of seconds instead.

 So this compromise will have to be retained somehow even if a proper 
target description is supplied by an RSP stub.  Using such a description 
will have the advantage of giving access to registers that were not 
included in the fixed format, e.g. shadow register sets.

  Maciej