[v2] gdb: xtensa: handle privileged registers

Message ID 20180507171121.7746-1-jcmvbkbc@gmail.com
State New, archived
Headers

Commit Message

Max Filippov May 7, 2018, 5:11 p.m. UTC
  xtensa GDB may be used with both bare-metal and linux-based
applications. In case of bare-metal application gdbserver is able to
provide information about all CPU registers: both unprivileged and
privileged. In case of linux-based application only a small subset of
privileged state is available. Currently xtensa GDB only expects
unprivileged registers in 'g' packets and it fails to communicate with
server that sends both privileged and unprivileged registers.

Allow bare-metal xtensa GDB to deal with both privileged and
unprivileged registers by initializing tdep->num_regs with the total
number of target CPU registers. Keep linux-based xtensa GDB
functionality as is by copying tdep->num_nopriv_regs to tdep->num_regs.

gdb/
2018-05-07  Max Filippov  <jcmvbkbc@gmail.com>

	* xtensa-linux-tdep.c (xtensa-tdep.h): New include.
	(xtensa_linux_init_abi): Limit tdep->num_regs by
	tdep->num_nopriv_regs.
	* xtensa-tdep.c (xtensa_derive_tdep): Calculate
	tdep->num_nopriv_regs and only copy it to tdep->num_regs if it's
	not initialized.
---
Changes v1->v2:
- expand description.

 gdb/xtensa-linux-tdep.c |  6 ++++++
 gdb/xtensa-tdep.c       | 12 ++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)
  

Comments

Pedro Alves May 9, 2018, 2:43 p.m. UTC | #1
Thanks for the updated patch with the rationale in the
commit log.  That helps.

On 05/07/2018 06:11 PM, Max Filippov wrote:
> xtensa GDB may be used with both bare-metal and linux-based
> applications. In case of bare-metal application gdbserver is able to
> provide information about all CPU registers: both unprivileged and
> privileged. In case of linux-based application only a small subset of
> privileged state is available. Currently xtensa GDB only expects
> unprivileged registers in 'g' packets and it fails to communicate with
> server that sends both privileged and unprivileged registers.
> 
> Allow bare-metal xtensa GDB to deal with both privileged and
> unprivileged registers by initializing tdep->num_regs with the total
> number of target CPU registers. Keep linux-based xtensa GDB
> functionality as is by copying tdep->num_nopriv_regs to tdep->num_regs.
> 
> gdb/
> 2018-05-07  Max Filippov  <jcmvbkbc@gmail.com>
> 
> 	* xtensa-linux-tdep.c (xtensa-tdep.h): New include.
> 	(xtensa_linux_init_abi): Limit tdep->num_regs by
> 	tdep->num_nopriv_regs.
> 	* xtensa-tdep.c (xtensa_derive_tdep): Calculate
> 	tdep->num_nopriv_regs and only copy it to tdep->num_regs if it's
> 	not initialized.

I have a feeling that this would be best address with xml
target descriptions, but I really don't know enough about
xtensa to say for sure.  This is OK with me as is.

Thanks,
Pedro Alves
  
Max Filippov May 9, 2018, 4:33 p.m. UTC | #2
On Wed, May 9, 2018 at 7:43 AM, Pedro Alves <palves@redhat.com> wrote:
> Thanks for the updated patch with the rationale in the
> commit log.  That helps.
>
> On 05/07/2018 06:11 PM, Max Filippov wrote:
>> xtensa GDB may be used with both bare-metal and linux-based
>> applications. In case of bare-metal application gdbserver is able to
>> provide information about all CPU registers: both unprivileged and
>> privileged. In case of linux-based application only a small subset of
>> privileged state is available. Currently xtensa GDB only expects
>> unprivileged registers in 'g' packets and it fails to communicate with
>> server that sends both privileged and unprivileged registers.
>>
>> Allow bare-metal xtensa GDB to deal with both privileged and
>> unprivileged registers by initializing tdep->num_regs with the total
>> number of target CPU registers. Keep linux-based xtensa GDB
>> functionality as is by copying tdep->num_nopriv_regs to tdep->num_regs.
>>
>> gdb/
>> 2018-05-07  Max Filippov  <jcmvbkbc@gmail.com>
>>
>>       * xtensa-linux-tdep.c (xtensa-tdep.h): New include.
>>       (xtensa_linux_init_abi): Limit tdep->num_regs by
>>       tdep->num_nopriv_regs.
>>       * xtensa-tdep.c (xtensa_derive_tdep): Calculate
>>       tdep->num_nopriv_regs and only copy it to tdep->num_regs if it's
>>       not initialized.
>
> I have a feeling that this would be best address with xml
> target descriptions, but I really don't know enough about
> xtensa to say for sure.  This is OK with me as is.

Thank you. Applied to master.

-- Max
  

Patch

diff --git a/gdb/xtensa-linux-tdep.c b/gdb/xtensa-linux-tdep.c
index a9b30c73f7bf..b62085de2fcb 100644
--- a/gdb/xtensa-linux-tdep.c
+++ b/gdb/xtensa-linux-tdep.c
@@ -18,6 +18,7 @@ 
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "xtensa-tdep.h"
 #include "osabi.h"
 #include "linux-tdep.h"
 #include "solib-svr4.h"
@@ -97,6 +98,11 @@  xtensa_linux_gdb_signal_to_target (struct gdbarch *gdbarch,
 static void
 xtensa_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (tdep->num_nopriv_regs < tdep->num_regs)
+    tdep->num_regs = tdep->num_nopriv_regs;
+
   linux_init_abi (info, gdbarch);
 
   set_solib_svr4_fetch_link_map_offsets
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index a1ecf5f56a42..01f96165dc8d 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -3145,16 +3145,12 @@  xtensa_derive_tdep (struct gdbarch_tdep *tdep)
 	max_size = rmap->byte_size;
       if (rmap->mask != 0 && tdep->num_regs == 0)
 	tdep->num_regs = n;
-      /* Find out out how to deal with priveleged registers.
-
-         if ((rmap->flags & XTENSA_REGISTER_FLAGS_PRIVILEGED) != 0
-              && tdep->num_nopriv_regs == 0)
-           tdep->num_nopriv_regs = n;
-      */
       if ((rmap->flags & XTENSA_REGISTER_FLAGS_PRIVILEGED) != 0
-	  && tdep->num_regs == 0)
-	tdep->num_regs = n;
+	  && tdep->num_nopriv_regs == 0)
+	tdep->num_nopriv_regs = n;
     }
+  if (tdep->num_regs == 0)
+    tdep->num_regs = tdep->num_nopriv_regs;
 
   /* Number of pseudo registers.  */
   tdep->num_pseudo_regs = n - tdep->num_regs;