[Patch] Microblaze: Port of Linux gdbserver

[Ajit Kumar Agarwal <ajit.kumar.agarwal@xilinx.com>]

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

Hello Pedro:

Please find the updated patch with feedback comments incorporated.

    Microblaze: Port of Linux gdbserver
    
    This patch is the port of Linux gdbserver.
    Tested with gdb regression testsuite with this patch of
    gdbserver.
    
    gdb/:
    2014-10-08  Ajit Agarwal  <ajitkum@xilinx.com>
    
        * configure.tgt (build_gdbserver): New Definition.
    
    gdb/gdbserver/:
    
        * gdbserver/Makefile.in (microblaze-linux.c): New target.
        * gdbserver/configure.srv (microblaze*-*-linux*): New target.
        * gdbserver/linux-microblaze-low.c: New file.
    
    Signed-off-by:Ajit Agarwal ajitkum@xilinx.com

Thanks & Regards
Ajit
-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Tuesday, September 30, 2014 5:14 PM
To: Ajit Kumar Agarwal; Michael Eager; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

On 09/23/2014 01:49 PM, Ajit Kumar Agarwal wrote:

>>>> >>> Note nothing is done with valid_p.  It's write-only.  Compare with other ports, like arm-tdep.c or mips-tdep.c.
>> > 
>> > Would look into this and will make the modification.
> Thanks.

I'd much prefer if we had that patch in the tree before accepting further patches that tweak things around register names, etc.  Could you send that (as an independent patch, in a new thread).

> +#define microblaze_num_regs	\
> +  (sizeof microblaze_regmap / sizeof microblaze_regmap[0])

#define microblaze_num_regs ARRAY_SIZE (microblaze_regmap)

> +
> +static int
> +microblaze_cannot_store_register (int regno) {
> +  if (microblaze_regmap[regno] == -1 || regno == 0)
> +    return 1;
> +
> +  return 0;
> +}
> +
> +static int
> +microblaze_cannot_fetch_register (int regno) {
> +  return 0;
> +}
> +
> +static CORE_ADDR
> +microblaze_get_pc (struct regcache *regcache) {
> +  unsigned long pc;
> +  collect_register_by_name (regcache, "rpc", &pc);

Empty line after declaration.  In several more places in the patch.
Please fix them all.

> +  return (CORE_ADDR) (pc);
> +}



> +  if (regno == 0)
> +    {
> +      unsigned long regbuf_0 = 0;
> +      /* Clobbering r0 so that it is always 0 as enforced by hardware.  */
> +      supply_register (regcache, regno, (const char*)&regbuf_0);

	supply_register_zeroed (regcache, regno);

> +    }
> +  else
> +    {
> +      if (size < sizeof (long))
> +        supply_register (regcache, regno, buf + sizeof (long) - size);
> +      else
> +        supply_register (regcache, regno, buf);
> +    }
> +}
> +
> +/* Provide only a fill function for the general register set.  ps_lgetregs
> +   will use this for NPTL support.  */
> +
> +static void microblaze_fill_gregset (struct regcache *regcache, void 
> +*buf)

Line break after "static void".  Function name goes on column 0:

static void
microblaze_fill_gregset (struct regcache *regcache, void *buf)

Please make sure that's correct throughout.


> +{
> +  int i;
> +
> +  for (i = 0; i < microblaze_num_regs; i++)
> +    microblaze_collect_ptrace_register (regcache, i,
> +                                        (char *) buf + 
> +microblaze_regmap[i]); }
> +
> +static void
> +microblaze_store_gregset (struct regcache *regcache, const void *buf) 
> +{
> +  int i;
> +  for (i = 0; i < microblaze_num_regs; i++)
> +    supply_register (regcache, i, (char *) buf + 
> +microblaze_regmap[i]); }
> +
> +#endif /* HAVE_PTRACE_GETREGS */
> +
> +static struct regset_info microblaze_regsets[] = { #ifdef 
> +HAVE_PTRACE_GETREGS

What's the #ifdef for?

Did this kernel port make it upstream without PTRACE_GETREGSET?
If there's support for that, can you please switch to using it?

PTRACE_GETREGS is supposed to an old way of doing things...

> +  { PTRACE_GETREGS, PTRACE_SETREGS, 0, 36 * sizeof (elf_gregset_t),
> +    GENERAL_REGS, microblaze_fill_gregset, microblaze_store_gregset 
> +},
> +  { 0, 0, 0, -1, -1, NULL, NULL },
> +#endif /* HAVE_PTRACE_GETREGS */


> +  { 0, 0, 0, -1, -1, NULL, NULL }
> +};
> +



> diff --git a/gdb/regformats/microblaze-with-stack-protect.dat 
> b/gdb/regformats/microblaze-with-stack-protect.dat
> index f71c111..e349b4a 100644
> --- a/gdb/regformats/microblaze-with-stack-protect.dat
> +++ b/gdb/regformats/microblaze-with-stack-protect.dat
> @@ -1,7 +1,7 @@
>  # DO NOT EDIT: generated from microblaze-with-stack-protect.xml
     ^^^^^^^^^^^
     ^^^^^^^^^^^
     ^^^^^^^^^^^
     ^^^^^^^^^^^
     ^^^^^^^^^^^

Please send a preparatory, independent, patch that updates features/Makefile instead and generates this file, in a new thread, with self-contained description, following the
checklist:
 https://sourceware.org/gdb/wiki/ContributionChecklist

>  name:microblaze_with_stack_protect
>  xmltarget:microblaze-with-stack-protect.xml
> -expedite:r1,pc
> +expedite:r1,rpc
>  32:r0
>  32:r1
>  32:r2
> -- 1.7.1
> 


Thanks,
Pedro Alves

[-- Attachment #2: 0001-Microblaze-Port-of-Linux-gdbserver.patch --]
[-- Type: application/octet-stream, Size: 10158 bytes --]

From 6588d2ab962bb1592acbfa674d781f89e6a865b2 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Wed, 8 Oct 2014 19:12:42 +0530
Subject: [PATCH] Microblaze: Port of Linux gdbserver

This patch is the port of Linux gdbserver.
Tested with gdb regression testsuite with this patch of
gdbserver.

gdb/:
2014-10-08  Ajit Agarwal  <ajitkum@xilinx.com>

	* configure.tgt (build_gdbserver): New Definition.

gdb/gdbserver/:

	* gdbserver/Makefile.in (microblaze-linux.c): New target.
	* gdbserver/configure.srv (microblaze*-*-linux*): New target.
	* gdbserver/linux-microblaze-low.c: New file.

Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
---
 gdb/configure.tgt                    |    1 +
 gdb/gdbserver/Makefile.in            |    4 +
 gdb/gdbserver/configure.srv          |    6 +
 gdb/gdbserver/linux-microblaze-low.c |  234 ++++++++++++++++++++++++++++++++++
 4 files changed, 245 insertions(+), 0 deletions(-)
 create mode 100644 gdb/gdbserver/linux-microblaze-low.c

diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 9aea728..e8cd3fa 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -343,6 +343,7 @@ microblaze*-linux-*|microblaze*-*-linux*)
 	gdb_target_obs="microblaze-tdep.o microblaze-linux-tdep.o microblaze-rom.o \
 			monitor.o dsrec.o solib-svr4.o symfile-mem.o linux-tdep.o"
 	gdb_sim=../sim/microblaze/libsim.a
+	build_gdbserver=yes
 	;;
 microblaze*-*-*)
 	# Target: Xilinx MicroBlaze running standalone
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 8b0318a..84873c1 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -154,6 +154,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/linux-ia64-low.c $(srcdir)/linux-low.c \
 	$(srcdir)/linux-m32r-low.c \
 	$(srcdir)/linux-m68k-low.c $(srcdir)/linux-mips-low.c \
+	$(srcdir)/linux-microblaze-low.c \
 	$(srcdir)/linux-nios2-low.c \
 	$(srcdir)/linux-ppc-low.c \
 	$(srcdir)/linux-s390-low.c \
@@ -366,6 +367,7 @@ clean:
 	rm -f amd64-mpx.c amd64-mpx-linux.c
 	rm -f amd64-avx512.c amd64-avx512-linux.c
 	rm -f i386-mmx.c i386-mmx-linux.c
+	rm -f microblaze-linux.c
 	rm -f x32.c x32-linux.c
 	rm -f x32-avx.c x32-avx-linux.c
 	rm -f x32-avx512.c x32-avx512-linux.c
@@ -636,6 +638,8 @@ mips64-linux.c : $(srcdir)/../regformats/mips64-linux.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/mips64-linux.dat mips64-linux.c
 mips64-dsp-linux.c : $(srcdir)/../regformats/mips64-dsp-linux.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/mips64-dsp-linux.dat mips64-dsp-linux.c
+microblaze-linux.c : $(srcdir)/../regformats/microblaze-with-stack-protect.dat $(regdat_sh)
+	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/microblaze-with-stack-protect.dat  microblaze-linux.c
 nios2-linux.c :	$(srcdir)/../regformats/nios2-linux.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/nios2-linux.dat nios2-linux.c
 powerpc-32.c : $(srcdir)/../regformats/rs6000/powerpc-32.dat $(regdat_sh)
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 679fc9f..a7b87aa 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -202,6 +202,12 @@ case "${target}" in
 			srv_linux_usrregs=yes
 			srv_linux_thread_db=yes
 			;;
+  microblaze*-*-linux*) srv_regobj=microblaze-linux.o
+			srv_tgtobj="$srv_linux_obj linux-microblaze-low.o"
+			srv_linux_usrregs=yes
+			srv_linux_regsets=yes
+			srv_linux_thread_db=yes
+			;;
   powerpc*-*-linux*)	srv_regobj="powerpc-32l.o"
 			srv_regobj="${srv_regobj} powerpc-altivec32l.o"
 			srv_regobj="${srv_regobj} powerpc-cell32l.o"
diff --git a/gdb/gdbserver/linux-microblaze-low.c b/gdb/gdbserver/linux-microblaze-low.c
new file mode 100644
index 0000000..c843c3a
--- /dev/null
+++ b/gdb/gdbserver/linux-microblaze-low.c
@@ -0,0 +1,234 @@
+/* GNU/Linux/Microblaze specific low level interface, for the remote server for
+   GDB.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "server.h"
+#include "linux-low.h"
+#include "gdb_proc_service.h"
+
+#include <asm/ptrace.h>
+#include <sys/procfs.h>
+
+void init_registers_microblaze_with_stack_protect (void);
+extern const struct target_desc *tdesc_microblaze_with_stack_protect;
+
+static int microblaze_regmap[] = {
+  PT_GPR(0),     PT_GPR(1),     PT_GPR(2),     PT_GPR(3),
+  PT_GPR(4),     PT_GPR(5),     PT_GPR(6),     PT_GPR(7),
+  PT_GPR(8),     PT_GPR(9),     PT_GPR(10),    PT_GPR(11),
+  PT_GPR(12),    PT_GPR(13),    PT_GPR(14),    PT_GPR(15),
+  PT_GPR(16),    PT_GPR(17),    PT_GPR(18),    PT_GPR(19),
+  PT_GPR(20),    PT_GPR(21),    PT_GPR(22),    PT_GPR(23),
+  PT_GPR(24),    PT_GPR(25),    PT_GPR(26),    PT_GPR(27),
+  PT_GPR(28),    PT_GPR(29),    PT_GPR(30),    PT_GPR(31),
+  PT_PC,         PT_MSR,        PT_EAR,        PT_ESR,
+  PT_FSR,        PT_BTR,        PT_PVR0,       PT_PVR1,
+  PT_PVR2,       PT_PVR3,       PT_PVR4,       PT_PVR5,
+  PT_PVR6,       PT_PVR7,       PT_PVR8,       PT_PVR9,
+  PT_PVR10,      PT_PVR11,      PT_EDR,        PT_PID,
+  PT_ZPR,        PT_TLBX,       PT_TLBSX,      PT_TLBLO,
+  PT_TLBHI,      PT_SLR,        PT_SHR
+};
+
+#define microblaze_num_regs ARRAY_SIZE (microblaze_regmap)
+
+static int
+microblaze_cannot_store_register (int regno)
+{
+  if (microblaze_regmap[regno] == -1 || regno == 0)
+    return 1;
+
+  return 0;
+}
+
+static int
+microblaze_cannot_fetch_register (int regno)
+{
+  return 0;
+}
+
+static CORE_ADDR
+microblaze_get_pc (struct regcache *regcache)
+{
+  unsigned long pc;
+
+  collect_register_by_name (regcache, "rpc", &pc);
+  return (CORE_ADDR) (pc);
+}
+
+static void
+microblaze_set_pc (struct regcache *regcache, CORE_ADDR pc)
+{
+  unsigned long newpc = pc;
+
+  supply_register_by_name (regcache, "rpc", &newpc);
+}
+
+static const unsigned long microblaze_breakpoint = 0xba0c0018;
+
+#define microblaze_breakpoint_len 4
+
+static int
+microblaze_breakpoint_at (CORE_ADDR where)
+{
+  unsigned long insn;
+
+  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
+
+  if (insn == microblaze_breakpoint)
+    return 1;
+
+  return 0;
+}
+
+static CORE_ADDR
+microblaze_reinsert_addr (struct regcache *regcache)
+{
+  unsigned long pc;
+
+  collect_register_by_name (regcache, "r15", &pc);
+  return pc;
+}
+
+static void
+microblaze_collect_ptrace_register (struct regcache *regcache,
+                                    int regno, char *buf)
+{
+  int size = register_size (regcache->tdesc, regno);
+
+  memset (buf, 0, sizeof (long));
+
+  if (size < sizeof (long))
+    collect_register (regcache, regno, buf + sizeof (long) - size);
+  else
+    collect_register (regcache, regno, buf);
+}
+
+static void
+microblaze_supply_ptrace_register (struct regcache *regcache,
+			           int regno, const char *buf)
+{
+  int i;
+  int size = register_size (regcache->tdesc, regno);
+
+  if (regno == 0)
+    {
+      unsigned long regbuf_0 = 0;
+      /* Clobbering r0 so that it is always 0 as enforced by hardware.  */
+      supply_register (regcache, regno, (const char*)&regbuf_0);
+    }
+  else
+    {
+      if (size < sizeof (long))
+        supply_register (regcache, regno, buf + sizeof (long) - size);
+      else
+        supply_register (regcache, regno, buf);
+    }
+}
+
+/* Provide only a fill function for the general register set.  ps_lgetregs
+   will use this for NPTL support.  */
+
+static void
+microblaze_fill_gregset (struct regcache *regcache, void *buf)
+{
+  int i;
+
+  for (i = 0; i < microblaze_num_regs; i++)
+    microblaze_collect_ptrace_register (regcache, i,
+                                        (char *) buf + microblaze_regmap[i]);
+}
+
+static void
+microblaze_store_gregset (struct regcache *regcache, const void *buf)
+{
+  int i;
+
+  for (i = 0; i < microblaze_num_regs; i++)
+    supply_register (regcache, i, (char *) buf + microblaze_regmap[i]);
+}
+
+static struct regset_info microblaze_regsets[] = {
+  { PTRACE_GETREGS, PTRACE_SETREGS, 0, 36 * sizeof (elf_gregset_t),
+    GENERAL_REGS, microblaze_fill_gregset, microblaze_store_gregset },
+  { 0, 0, 0, -1, -1, NULL, NULL },
+  { 0, 0, 0, -1, -1, NULL, NULL }
+};
+
+static struct regsets_info microblaze_regsets_info = {
+  microblaze_regsets, /* regsets */
+  0, /* num_regsets */
+  NULL, /* disabled_regsets */
+};
+
+static struct usrregs_info microblaze_usrregs_info = {
+  microblaze_num_regs,
+  microblaze_regmap,
+};
+
+static struct regs_info regs_info = {
+  NULL, /* regset_bitmap */
+  &microblaze_usrregs_info,
+  &microblaze_regsets_info
+};
+
+static const struct regs_info *
+microblaze_regs_info (void)
+{
+  return &regs_info;
+}
+
+static void
+microblaze_arch_setup (void)
+{
+  current_process ()->tdesc = tdesc_microblaze_with_stack_protect;
+}
+
+struct linux_target_ops the_low_target = {
+  microblaze_arch_setup,
+  microblaze_regs_info,
+  microblaze_cannot_fetch_register,
+  microblaze_cannot_store_register,
+  NULL, /* fetch_register */
+  microblaze_get_pc,
+  microblaze_set_pc,
+  (const unsigned char *) &microblaze_breakpoint,
+  microblaze_breakpoint_len,
+  microblaze_reinsert_addr,
+  0,
+  microblaze_breakpoint_at,
+  NULL,
+  NULL,
+  NULL,
+  NULL,
+  NULL,
+  microblaze_collect_ptrace_register,
+  microblaze_supply_ptrace_register,
+  NULL,
+  NULL,
+  NULL,
+  NULL,
+};
+
+void
+initialize_low_arch (void)
+{
+  init_registers_microblaze_with_stack_protect ();
+
+  initialize_regsets_info (&microblaze_regsets_info);
+}
-- 
1.7.1