OpenRISC gdb port

Quoted 4 lines

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Franck Jullien <[email protected]>
Date: Mon, 24 Apr 2017 21:52:51 +0900

Review

Quoted 2 lines

(quoted diff) +2 -0

+The OpenRISC 1000 provides a free and open RISC instruction set
+architecture.

"Free" is a superset of "open", so I think you should leave only the former in the text. (It will also help us not to get in trouble with RMS.)

OK for the documentation part, with the above fixed.

gdb/doc/gdb.texinfo +30 -0

+
+ at kindex target sim
+ at item target sim
+
+Runs the builtin CPU simulator which can run very basic
+programs but does not support most hardware functions like MMU.
+For more complex use cases the user is advised to run an external
+target, and connect using @samp{target remote}.
+
+Example: @code{target sim}
+
+ at end table
+
 @node PowerPC Embedded
 @subsection PowerPC Embedded

@@ -41088,6 +41131,7 @@ registers using the capitalization used in the description.
 * M68K Features::
 * NDS32 Features::
 * Nios II Features::
+* OpenRISC 1000 Features::
 * PowerPC Features::
 * S/390 and System z Features::
 * Sparc Features::

@@ -41374,6 +41418,32 @@ targets.  It should contain the 32 core registers (@samp{zero},
 @samp{pc}, and the 16 control registers (@samp{status} through
 @samp{mpuacc}).
 
+ at node OpenRISC 1000 Features
+ at subsection Openrisc 1000 Features
+ at cindex target descriptions, OpenRISC 1000 features
+
+The @samp{org.gnu.gdb.or1k.group0} feature is required for OpenRISC 1000
+targets.  It should contain the 32 general purpose registers (@samp{r0}
+through @samp{r31}), @samp{ppc}, @samp{npc} and @samp{sr}.
+
+Along with the default reggroups like @samp{system} and @samp{general}
+provided by @value{GDBN}, OpenRISC targets can use the following reggroups
+to group their many registers:
+
+ at smallexample
+ Group      Type
+ immu       user
+ dmmu       user

gdb/testsuite/ChangeLog:

2017-04-20 Stafford Horne <[email protected]>

* gdb.xml/tdesc-regs.exp: Add or1k support.

2017-02-12 Stafford Horne <[email protected]>

* configure.ac: remove logic adding gdb to noconfigsdirs for or1k

* configure: regenerate

tdesc_register_in_reggroup_p in now able to handle arbitrary groups. This is useful when groups are created while the target descriptor file is received from the remote.

This can be the case of a soft core target processor where registers/groups can change.

gdb/ChangeLog:

2013-02-15 Franck Jullien <[email protected]>

* target-descriptions.c (tdesc_register_in_reggroup_p): Support arbitrary strings.

Quoted 11 lines · Stafford Horne

tdesc_register_in_reggroup_p in now able to handle arbitrary groups. This is useful when groups are created while the target descriptor file is received from the remote.

This can be the case of a soft core target processor where registers/groups can change.

gdb/ChangeLog:

2013-02-15 Franck Jullien <[email protected]>

Quoted 2 lines

* target-descriptions.c (tdesc_register_in_reggroup_p): Support arbitrary strings.

I am not sure what do you want to change and why do you need this change? I didn't see it in the previous version.

If you want to control what registers are displayed, you can follow the way nds32 used, see nds32-tdep.c:nds32_register_reggroup_p.

gdb/doc/gdb.texinfo As usual, you can inquire about the @code{mipsfpu} variable with · line 22318

Quoted 10 lines · Stafford Horne

+ at kindex target remote
+ at item target remote

gdb/doc/gdb.texinfo +8 -0

+
+Connect to a remote OpenRISC 1000 target.  This is supported by
+ at dfn{Or1ksim}, the OpenRISC 1000 architectural simulator as well as QEMU,
+Verilator and Icarus Verilog simulators.  @dfn{Remote serial protocol}
+servers, such as OpenOCD, are also available to drive various hardware
+implementations via JTAG.
+
+Example: @code{target remote :51000}

Do we need to document "target remote"? which is not specific to any particular configurations.

Quoted 48 lines

gdb/doc/gdb.texinfo +34 -0

+
+ at kindex target sim
+ at item target sim
+
+Runs the builtin CPU simulator which can run very basic
+programs but does not support most hardware functions like MMU.
+For more complex use cases the user is advised to run an external
+target, and connect using @samp{target remote}.
+
+Example: @code{target sim}
+
+ at end table
+
 @node PowerPC Embedded
 @subsection PowerPC Embedded

@@ -41088,6 +41131,7 @@ registers using the capitalization used in the description.
 * M68K Features::
 * NDS32 Features::
 * Nios II Features::
+* OpenRISC 1000 Features::
 * PowerPC Features::
 * S/390 and System z Features::
 * Sparc Features::

@@ -41374,6 +41418,32 @@ targets.  It should contain the 32 core registers (@samp{zero},
 @samp{pc}, and the 16 control registers (@samp{status} through
 @samp{mpuacc}).
 
+ at node OpenRISC 1000 Features
+ at subsection Openrisc 1000 Features
+ at cindex target descriptions, OpenRISC 1000 features
+
+The @samp{org.gnu.gdb.or1k.group0} feature is required for OpenRISC 1000
+targets.  It should contain the 32 general purpose registers (@samp{r0}
+through @samp{r31}), @samp{ppc}, @samp{npc} and @samp{sr}.
+
+Along with the default reggroups like @samp{system} and @samp{general}
+provided by @value{GDBN}, OpenRISC targets can use the following reggroups
+to group their many registers:
+
+ at smallexample
+ Group      Type
+ immu       user
+ dmmu       user
+ icache     user
+ dcache     user
+ pic        user
+ timer      user

gdb/doc/gdb.texinfo targets. It should contain the 32 core registers (@samp{zero}, · line 41446

Quoted 6 lines

gdb/doc/gdb.texinfo +6 -0

+ power      user
+ perf       user
+ mac        user
+ debug      user
+ at end smallexample
+

Why do you need to document the reggroups?

gdb/or1k-tdep.c line 1290

Quoted 3 lines

gdb/or1k-tdep.c new file +3 -0

+
+extern initialize_file_ftype _initialize_or1k_tdep; /* -Wmissing-prototypes */
+

This is no longer needed, because GDB is moved to C++.

Otherwise, the code is good to me.

Signature · 1 line

Yao (齐尧)

This change is to also allow returning true in the case that the register group was registered with gdb with reggroup_add(). This seems like something that will be generally required so it was added to the target-descriptions definition. If you think not I can make it for openrisc only, but I think it makes more sense in tdesc.

This allows the command like, the below to return the registers for the named reggroup.

info reg system

I understand this is nothing new, but this tells to the users some extra details about using target remote for OpenRISC (example which platforms support it). Also, this is consistent with some other targets like Microblaze.

Quoted 50 lines

Quoted 50 lines

gdb/doc/gdb.texinfo +13 -0

+
+ at kindex target sim
+ at item target sim
+
+Runs the builtin CPU simulator which can run very basic
+programs but does not support most hardware functions like MMU.
+For more complex use cases the user is advised to run an external
+target, and connect using @samp{target remote}.
+
+Example: @code{target sim}
+
+ at end table
+
 @node PowerPC Embedded
 @subsection PowerPC Embedded

@@ -41088,6 +41131,7 @@ registers using the capitalization used in

the description.
* M68K Features::
* NDS32 Features::
* Nios II Features::

gdb/doc/gdb.texinfo +21 -0

+* OpenRISC 1000 Features::
 * PowerPC Features::
 * S/390 and System z Features::
 * Sparc Features::

@@ -41374,6 +41418,32 @@ targets.  It should contain the 32 core
 registers (@samp{zero},
 @samp{pc}, and the 16 control registers (@samp{status} through
 @samp{mpuacc}).
 
+ at node OpenRISC 1000 Features
+ at subsection Openrisc 1000 Features
+ at cindex target descriptions, OpenRISC 1000 features
+
+The @samp{org.gnu.gdb.or1k.group0} feature is required for OpenRISC 1000
+targets.  It should contain the 32 general purpose registers (@samp{r0}
+through @samp{r31}), @samp{ppc}, @samp{npc} and @samp{sr}.
+
+Along with the default reggroups like @samp{system} and @samp{general}
+provided by @value{GDBN}, OpenRISC targets can use the following reggroups
+to group their many registers:
+
+ at smallexample
+ Group      Type
+ immu       user
+ dmmu       user
+ icache     user
+ dcache     user
+ pic        user
+ timer      user

Quoted 8 lines

gdb/doc/gdb.texinfo targets. It should contain the 32 core registers (@samp{zero}, · line 41446

Quoted 6 lines

gdb/doc/gdb.texinfo +6 -0

+ power      user
+ perf       user
+ mac        user
+ debug      user
+ at end smallexample
+

Why do you need to document the reggroups?

These register groups can be used by the target description features. If not documented one would have to look into the code. In general arbitrary groups are not allowed by features. This is also related to patch 1/5.

Quoted 5 lines · Stafford Horne

I understand this is nothing new, but this tells to the users some extra details about using target remote for OpenRISC (example which platforms support it). Also, this is consistent with some other targets like Microblaze.

All the targets listed in "Embedded Processors" do have "target remote". We don't have to document it for every one. IMO, we need to remove "target remote" from Microblaze part too.

Quoted 59 lines

Quoted 58 lines

Quoted 46 lines

gdb/doc/gdb.texinfo +13 -0

+
+ at kindex target sim
+ at item target sim
+
+Runs the builtin CPU simulator which can run very basic
+programs but does not support most hardware functions like MMU.
+For more complex use cases the user is advised to run an external
+target, and connect using @samp{target remote}.
+
+Example: @code{target sim}
+
+ at end table
+
 @node PowerPC Embedded
 @subsection PowerPC Embedded

@@ -41088,6 +41131,7 @@ registers using the capitalization used in

the description.
* M68K Features::
* NDS32 Features::
* Nios II Features::

gdb/doc/gdb.texinfo +17 -0

+* OpenRISC 1000 Features::
 * PowerPC Features::
 * S/390 and System z Features::
 * Sparc Features::

@@ -41374,6 +41418,32 @@ targets.  It should contain the 32 core
 registers (@samp{zero},
 @samp{pc}, and the 16 control registers (@samp{status} through
 @samp{mpuacc}).
 
+ at node OpenRISC 1000 Features
+ at subsection Openrisc 1000 Features
+ at cindex target descriptions, OpenRISC 1000 features
+
+The @samp{org.gnu.gdb.or1k.group0} feature is required for OpenRISC 1000
+targets.  It should contain the 32 general purpose registers (@samp{r0}
+through @samp{r31}), @samp{ppc}, @samp{npc} and @samp{sr}.
+
+Along with the default reggroups like @samp{system} and @samp{general}
+provided by @value{GDBN}, OpenRISC targets can use the following reggroups
+to group their many registers:
+
+ at smallexample
+ Group      Type
+ immu       user
+ dmmu       user

gdb/doc/gdb.texinfo targets. It should contain the 32 core · line 41447

Quoted 10 lines

gdb/doc/gdb.texinfo +10 -0

+ icache     user
+ dcache     user
+ pic        user
+ timer      user
+ power      user
+ perf       user
+ mac        user
+ debug      user
+ at end smallexample
+

Why do you need to document the reggroups?

Quoted 3 lines

These register groups can be used by the target description features. If not documented one would have to look into the code. In general arbitrary groups are not allowed by features. This is also related to patch 1/5.

"maintenance print reggroups" can tell the reggroups, so don't need to document them.

Signature · 1 line

Yao (齐尧)

gdb/target-descriptions.c tdesc_register_in_reggroup_p (struct gdbarch *gdbarch, int regno, · line 1123

Quoted 8 lines · Stafford Horne

 
       if (reggroup == general_reggroup)

gdb/target-descriptions.c +4 -0

 	return general_p;
+
+      if (strcmp (reg->group, reggroup_name (reggroup)) == 0)
+	return 1;
+
     }

I read the patch again, and find that we can replace the whole block in "if (reg != NULL && reg->group != NULL)" with the code this patch adds, like this,

  if (reg != NULL && reg->group != NULL
      && (strcmp (reg->group, reggroup_name (reggroup)) == 0))
     return 1;

Signature · 1 line

Yao (齐尧)

Quoted 5 lines · Stafford Horne

This change is to also allow returning true in the case that the register group was registered with gdb with reggroup_add(). This seems like something that will be generally required so it was added to the target-descriptions definition. If you think not I can make it for openrisc only, but I think it makes more sense in tdesc.

Thanks for the explanation.

Quoted 5 lines

This allows the command like, the below to return the registers for the named reggroup.

info reg system

Some bits are missing,

- A test case, in which we can define some reggroups "foo" and test expected "foo" is shown in the output of "maintenance print reggroups". You can add reggroups in gdb.xml/extra-regs.xml.

- A test case with live inferior, get a list of support reggroups from the output of "maint print reggroups", pass each reggroup to "info reg $group", and test there is no error.

- Document "info registers".
https://sourceware.org/gdb/current/onlinedocs/gdb/Registers.html
doesn't document the usage "info reg GROUP". We need to add it.

- Add a news entry, because your patch adds a user visible change.