public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [Patch] Microblaze: Port of Linux gdbserver
@ 2014-10-08 13:52 Ajit Kumar Agarwal
  2014-10-09 16:29 ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-08 13:52 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

[-- 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


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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-08 13:52 [Patch] Microblaze: Port of Linux gdbserver Ajit Kumar Agarwal
@ 2014-10-09 16:29 ` Michael Eager
  2014-10-09 18:54   ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-10-09 16:29 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 10/08/14 06:51, Ajit Kumar Agarwal wrote:

> Please find the updated patch with feedback comments incorporated.

Please don't top post and please trim email.

Please respond to questions/comments inline.  This makes it easier
to tell whether all issues have been addressed.

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

+#define microblaze_breakpoint_len 4

Use CAPS for macros.

+  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
+
+  if (insn == microblaze_breakpoint)

Why use the explicit length rather than the macro you just defined?
Why not use sizeof (insn)?

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

Please address issues with previous patches before moving
on to submit dependent patches.

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

Please answer all questions.

Pedro:
> PTRACE_GETREGS is supposed to an old way of doing things...

And address all comments.

Pedro:
>diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
...
> 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

Preparatory means that the patch should be submitted
before the current patch.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-09 16:29 ` Michael Eager
@ 2014-10-09 18:54   ` Ajit Kumar Agarwal
  2014-10-09 23:42     ` Michael Eager
  2014-10-15 13:27     ` Pedro Alves
  0 siblings, 2 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-09 18:54 UTC (permalink / raw)
  To: Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Thursday, October 09, 2014 9:59 PM
To: Ajit Kumar Agarwal; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver

On 10/08/14 06:51, Ajit Kumar Agarwal wrote:

> Please find the updated patch with feedback comments incorporated.

>>Please don't top post and please trim email.
To send the patches after incorporating the comments, Is there any other way of sending the patches without top post?
I will make sure that I will trim the mail.

>>Please respond to questions/comments inline.  This makes it easier to tell whether all issues have been addressed.

Sure. Please find my comments inlined below.

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

+#define microblaze_breakpoint_len 4

>>Use CAPS for macros.

The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.

+  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
+
+  if (insn == microblaze_breakpoint)

>>Why use the explicit length rather than the macro you just defined?
>>Why not use sizeof (insn)?

To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips  4 is used  and in the ARM target for the THUMB_ARM 2 is used  and for the ARM Mode code 4 is used.  

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

>>Please address issues with previous patches before moving on to submit dependent patches.

I have already send  the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments.

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

>>Please answer all questions.

Sure.  The Kernel code(ptrace.h) for Microblaze doesn't have upstream code without PTRACE_GETREGSET.

Pedro:
> PTRACE_GETREGS is supposed to an old way of doing things...

>>And address all comments.

The Microblaze Kernel code PTRACE_GETREGS is always defined and there is no conditional compilation which is without the PTRACE_GETREGS. So I agree with Pedro comment of not using #ifdef PTRACE_GETREGS and in the patch submitted I have removed if #ifdef PTRACE_GETREGS which is not required.

Pedro:
>diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
...
> 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

>>Preparatory means that the patch should be submitted before the current patch.

I will be sending this patch soon.

Thanks & Regards
Ajit

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-09 18:54   ` Ajit Kumar Agarwal
@ 2014-10-09 23:42     ` Michael Eager
  2014-10-13 16:00       ` Ajit Kumar Agarwal
  2014-10-15 13:27     ` Pedro Alves
  1 sibling, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-10-09 23:42 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 10/09/14 11:54, Ajit Kumar Agarwal wrote:
>
> To send the patches after incorporating the comments, Is there any other way of sending the patches without top post?

After you address comments, include the changelog at the end and attach
the patch (unless it is just a few lines).  That way we can tell how
you responded to each comment.

> +#define microblaze_breakpoint_len 4
>
>>> Use CAPS for macros.
>
> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.

Let's follow the GNU coding standard, even if some other targets haven't.

> +  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
> +
> +  if (insn == microblaze_breakpoint)
>
>>> Why use the explicit length rather than the macro you just defined?
>>> Why not use sizeof (insn)?
>
> To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips  4 is used  and in the ARM target for the THUMB_ARM 2 is used  and for the ARM Mode code 4 is used.

Let's follow good coding practice, even if there have been lapses in the past.
Unless there is some particular relevance to instruction length on MIPS or
ARM/Thumb, let's stick to what is relevant to MicroBlaze.

> Pedro:
>> 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).
>
>>> Please address issues with previous patches before moving on to submit dependent patches.
>
> I have already send  the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments.

I haven't seen this patch.  Please let me know when you posted it, or
send me a link to it in the mailing list archive.

If you submit a patch which depends on previously submitted patches
which have not been accepted, the new patch will not be accepted.
Please don't submit dependent patches until all prior prerequisite
patches are accepted.


> Pedro:
>> diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
> ...
>> 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
>
>>> Preparatory means that the patch should be submitted before the current patch.
>
> I will be sending this patch soon.

OK.  As mentioned above, please do not resubmit this patch until that
patch is submitted and accepted.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-09 23:42     ` Michael Eager
@ 2014-10-13 16:00       ` Ajit Kumar Agarwal
  2014-10-13 17:49         ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-13 16:00 UTC (permalink / raw)
  To: Michael Eager, Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com] 
Sent: Friday, October 10, 2014 5:12 AM
To: Ajit Kumar Agarwal; Michael Eager; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver

On 10/09/14 11:54, Ajit Kumar Agarwal wrote:
>
> To send the patches after incorporating the comments, Is there any other way of sending the patches without top post?

After you address comments, include the changelog at the end and attach the patch (unless it is just a few lines).  That way we can tell how you responded to each comment.

> +#define microblaze_breakpoint_len 4
>
>>> Use CAPS for macros.
>
> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.

>Let's follow the GNU coding standard, even if some other targets haven't.

https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html

Here is the mailing list archive that mentions when not to use ALL_CAPS for Macros.

> +  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
> +
> +  if (insn == microblaze_breakpoint)
>
>>> Why use the explicit length rather than the macro you just defined?
>>> Why not use sizeof (insn)?
>
> To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips  4 is used  and in the ARM target for the THUMB_ARM 2 is used  and for the ARM Mode code 4 is used.

>Let's follow good coding practice, even if there have been lapses in the past.
Unless there is some particular relevance to instruction length on MIPS or ARM/Thumb, let's stick to what is relevant to MicroBlaze.

Ok.

> Pedro:
>> 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).
>
>>> Please address issues with previous patches before moving on to submit dependent patches.
>
> I have already send  the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments.

>I haven't seen this patch.  Please let me know when you posted it, or send me a link to it in the mailing list archive.


>If you submit a patch which depends on previously submitted patches which have not been accepted, the new patch will not be accepted.
Please don't submit dependent patches until all prior prerequisite patches are accepted.

http://sourceware-org.1504.n7.nabble.com/PATCH-Microblaze-Reject-invalid-target-descriptions-td285201.html


> Pedro:
>> diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
> ...
>> 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
>
>>> Preparatory means that the patch should be submitted before the current patch.
>
> I will be sending this patch soon.

OK.  As mentioned above, please do not resubmit this patch until that patch is submitted and accepted.

http://sourceware-org.1504.n7.nabble.com/PATCH-Microblaze-Replace-microblaze-expedite-from-pc-to-rpc-td285649.html

Thanks & Regards
Ajit

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-13 16:00       ` Ajit Kumar Agarwal
@ 2014-10-13 17:49         ` Michael Eager
  2014-10-14  3:03           ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-10-13 17:49 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 10/13/14 09:00, Ajit Kumar Agarwal wrote:
>

>> +#define microblaze_breakpoint_len 4
>>
>>>> Use CAPS for macros.
>>
>> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
>
>> Let's follow the GNU coding standard, even if some other targets haven't.
>
> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html

To quote your first reference:

"I actually think it's a mistake to spell function-like macros in
ALL_CAPS, precisely because that makes changing back-and-forth unduly
disruptive."

To quote the second reference:

"ALL_CAPS in cases when it's important to be aware that it's a macro,
but when we make "wrappers" for efficiency purposes where we basically
want to pretend it's a function,".

This is not a function-like macro.  In this case, it is important to
be aware that this is a macro, not a variable.


AGAIN, please follow the GNU Coding Standard.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-13 17:49         ` Michael Eager
@ 2014-10-14  3:03           ` Ajit Kumar Agarwal
  2014-10-14 15:07             ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-14  3:03 UTC (permalink / raw)
  To: Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Monday, October 13, 2014 11:20 PM
To: Ajit Kumar Agarwal; Michael Eager; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver

On 10/13/14 09:00, Ajit Kumar Agarwal wrote:
>

>> +#define microblaze_breakpoint_len 4
>>
>>>> Use CAPS for macros.
>>
>> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
>
>> Let's follow the GNU coding standard, even if some other targets haven't.
>
> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html

>To quote your first reference:

>"I actually think it's a mistake to spell function-like macros in ALL_CAPS, precisely because that makes changing back-and-forth unduly disruptive."

>To quote the second reference:

>"ALL_CAPS in cases when it's important to be aware that it's a macro, but when we make "wrappers" for efficiency purposes where we basically want to >pretend it's a function,".

>This is not a function-like macro.  In this case, it is important to be aware that this is a macro, not a variable.

The macro microblaze_breakpoint_len is initialized in the struct linux_target_ops the_low_target.  This makes it as a wrapper and the above quote is valid.  
Still the CAPS is required for the above macro? 

AGAIN, please follow the GNU Coding Standard.

Thanks & Regards
Ajit
-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-14  3:03           ` Ajit Kumar Agarwal
@ 2014-10-14 15:07             ` Michael Eager
  2014-10-14 15:33               ` Ajit Kumar Agarwal
  2014-10-14 15:42               ` Ajit Kumar Agarwal
  0 siblings, 2 replies; 59+ messages in thread
From: Michael Eager @ 2014-10-14 15:07 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 10/13/14 20:03, Ajit Kumar Agarwal wrote:
>
>

> On 10/13/14 09:00, Ajit Kumar Agarwal wrote:
>>
>
>>> +#define microblaze_breakpoint_len 4
>>>
>>>>> Use CAPS for macros.
>>>
>>> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
>>
>>> Let's follow the GNU coding standard, even if some other targets haven't.
>>
>> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
>> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html
>
>> To quote your first reference:
>
>> "I actually think it's a mistake to spell function-like macros in ALL_CAPS, precisely because that makes changing back-and-forth unduly disruptive."
>
>> To quote the second reference:
>
>> "ALL_CAPS in cases when it's important to be aware that it's a macro, but when we make "wrappers" for efficiency purposes where we basically want to >pretend it's a function,".
>
>> This is not a function-like macro.  In this case, it is important to be aware that this is a macro, not a variable.
>
> The macro microblaze_breakpoint_len is initialized in the struct linux_target_ops the_low_target.  This makes it as a wrapper and the above quote is valid.
> Still the CAPS is required for the above macro?

You clearly do not have any understanding of what a wrapper is.
A wrapper macro is one which looks like a function and calls
another function or macro, passing it some arguments.

Please read a definition of "wrapper" or "function-like macro":
http://programmers.stackexchange.com/questions/142069/using-macro-as-an-abstraction-layer
https://gcc.gnu.org/onlinedocs/cpp/Function-like-Macros.html

microblaze_breakpoint_len is not initialized in any struct or anywhere
else.  It is a preprocessor macro definition.  In C, only variables can be
initialized, and that cannot happen in a struct.

microblaze_breakpoint_len is used in an initializer for an instance
of struct linux_target_ops.

In general, macro definitions like this one should be at the top of a
file, not buried in the middle of the file.

You appear to be unfamiliar with common terminology like wrapper, function-
like macro, or initialize, or what can or cannot be done within a struct. I
suggest you study C programming.  There are a number of books on advanced
C programming available online.

> AGAIN, please follow the GNU Coding Standard.

Please make sure when replying to email that you do not confuse your
responses with the text you are responding to.  I don't know how
you are reading or submitting email, but just about every message
is garbled.

USE CAPS FOR MACROS.  Is that clear enough?


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-14 15:07             ` Michael Eager
@ 2014-10-14 15:33               ` Ajit Kumar Agarwal
  2014-10-14 15:42               ` Ajit Kumar Agarwal
  1 sibling, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-14 15:33 UTC (permalink / raw)
  To: Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Tuesday, October 14, 2014 8:37 PM
To: Ajit Kumar Agarwal; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver

On 10/13/14 20:03, Ajit Kumar Agarwal wrote:
>
>

> On 10/13/14 09:00, Ajit Kumar Agarwal wrote:
>>
>
>>> +#define microblaze_breakpoint_len 4
>>>
>>>>> Use CAPS for macros.
>>>
>>> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
>>
>>> Let's follow the GNU coding standard, even if some other targets haven't.
>>
>> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
>> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html
>
>> To quote your first reference:
>
>> "I actually think it's a mistake to spell function-like macros in ALL_CAPS, precisely because that makes changing back-and-forth unduly disruptive."
>
>> To quote the second reference:
>
>> "ALL_CAPS in cases when it's important to be aware that it's a macro, but when we make "wrappers" for efficiency purposes where we basically want to >pretend it's a function,".
>
>> This is not a function-like macro.  In this case, it is important to be aware that this is a macro, not a variable.
>
> The macro microblaze_breakpoint_len is initialized in the struct linux_target_ops the_low_target.  This makes it as a wrapper and the above quote is valid.
> Still the CAPS is required for the above macro?

>>You clearly do not have any understanding of what a wrapper is.
>>A wrapper macro is one which looks like a function and calls another function or macro, passing it some arguments.

>>Please read a definition of "wrapper" or "function-like macro":
>>http://programmers.stackexchange.com/questions/142069/using-macro-as-an-abstraction-layer
>>https://gcc.gnu.org/onlinedocs/cpp/Function-like-Macros.html

>>microblaze_breakpoint_len is not initialized in any struct or anywhere else.  It is a preprocessor macro definition.  In C, only variables can be initialized, and >>that cannot happen in a struct.

>>microblaze_breakpoint_len is used in an initializer for an instance of struct linux_target_ops.

>>In general, macro definitions like this one should be at the top of a file, not buried in the middle of the file.

>>You appear to be unfamiliar with common terminology like wrapper, function- like macro, or initialize, or what can or cannot be done within a struct. I >>suggest you study C programming.  There are a number of books on advanced C programming available online.

I am well aware and adept with the above knowledge and I am afraid I won't be needing your guidance as far as this is concerned. 

> AGAIN, please follow the GNU Coding Standard.

Please make sure when replying to email that you do not confuse your responses with the text you are responding to.  I don't know how you are reading or submitting email, but just about every message is garbled.

USE CAPS FOR MACROS.  Is that clear enough?


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-14 15:07             ` Michael Eager
  2014-10-14 15:33               ` Ajit Kumar Agarwal
@ 2014-10-14 15:42               ` Ajit Kumar Agarwal
  1 sibling, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-14 15:42 UTC (permalink / raw)
  To: Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Ajit Kumar Agarwal 
Sent: Tuesday, October 14, 2014 9:03 PM
To: 'Michael Eager'; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch] Microblaze: Port of Linux gdbserver



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com]
Sent: Tuesday, October 14, 2014 8:37 PM
To: Ajit Kumar Agarwal; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver

On 10/13/14 20:03, Ajit Kumar Agarwal wrote:
>
>

> On 10/13/14 09:00, Ajit Kumar Agarwal wrote:
>>
>
>>> +#define microblaze_breakpoint_len 4
>>>
>>>>> Use CAPS for macros.
>>>
>>> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
>>
>>> Let's follow the GNU coding standard, even if some other targets haven't.
>>
>> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
>> https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html
>
>> To quote your first reference:
>
>> "I actually think it's a mistake to spell function-like macros in ALL_CAPS, precisely because that makes changing back-and-forth unduly disruptive."
>
>> To quote the second reference:
>
>> "ALL_CAPS in cases when it's important to be aware that it's a macro, but when we make "wrappers" for efficiency purposes where we basically want to >pretend it's a function,".
>
>> This is not a function-like macro.  In this case, it is important to be aware that this is a macro, not a variable.
>
> The macro microblaze_breakpoint_len is initialized in the struct linux_target_ops the_low_target.  This makes it as a wrapper and the above quote is valid.
> Still the CAPS is required for the above macro?

>>You clearly do not have any understanding of what a wrapper is.
>>A wrapper macro is one which looks like a function and calls another function or macro, passing it some arguments.

>>Please read a definition of "wrapper" or "function-like macro":
>>http://programmers.stackexchange.com/questions/142069/using-macro-as-a
>>n-abstraction-layer 
>>https://gcc.gnu.org/onlinedocs/cpp/Function-like-Macros.html

>>microblaze_breakpoint_len is not initialized in any struct or anywhere else.  It is a preprocessor macro definition.  In C, only variables can be initialized, and >>that cannot happen in a struct.

>>microblaze_breakpoint_len is used in an initializer for an instance of struct linux_target_ops.

>>In general, macro definitions like this one should be at the top of a file, not buried in the middle of the file.

>>You appear to be unfamiliar with common terminology like wrapper, function- like macro, or initialize, or what can or cannot be done within a struct. I >>suggest you study C programming.  There are a number of books on advanced C programming available online.

Ajit: I am well aware and adept with the above knowledge and I am afraid I won't be needing your guidance as far as this is concerned. 
         In addition to this I hope not to expect such strong statements from your end in future.

> AGAIN, please follow the GNU Coding Standard.

Please make sure when replying to email that you do not confuse your responses with the text you are responding to.  I don't know how you are reading or submitting email, but just about every message is garbled.

USE CAPS FOR MACROS.  Is that clear enough?


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-09 18:54   ` Ajit Kumar Agarwal
  2014-10-09 23:42     ` Michael Eager
@ 2014-10-15 13:27     ` Pedro Alves
  2014-10-17 19:22       ` Ajit Kumar Agarwal
  2014-11-26 12:13       ` Ajit Kumar Agarwal
  1 sibling, 2 replies; 59+ messages in thread
From: Pedro Alves @ 2014-10-15 13:27 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 10/09/2014 07:54 PM, Ajit Kumar Agarwal wrote:
> 
> Pedro:
>> > Did this kernel port make it upstream without PTRACE_GETREGSET?
>> > If there's support for that, can you please switch to using it?
>>> >>Please answer all questions.
> Sure.  The Kernel code(ptrace.h) for Microblaze doesn't have upstream code without PTRACE_GETREGSET.
> 
> Pedro:
>> > PTRACE_GETREGS is supposed to an old way of doing things...
>>> >>And address all comments.
> The Microblaze Kernel code PTRACE_GETREGS is always defined and there is no conditional compilation which is without the PTRACE_GETREGS. So I agree with Pedro comment of not using #ifdef PTRACE_GETREGS and in the patch submitted I have removed if #ifdef PTRACE_GETREGS which is not required.

PTRACE_GETREGSET != PTRACE_GETREGS

I'm asking for using the PTRACE_GETREGSET instead of PTRACE_GETREGS
in new ports.  See background here:

  https://sourceware.org/ml/archer/2010-q3/msg00193.html

Thanks,
Pedro Alves

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-15 13:27     ` Pedro Alves
@ 2014-10-17 19:22       ` Ajit Kumar Agarwal
  2014-12-15 18:02         ` Pedro Alves
  2014-11-26 12:13       ` Ajit Kumar Agarwal
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-17 19:22 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

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



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Wednesday, October 15, 2014 6:57 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 10/09/2014 07:54 PM, Ajit Kumar Agarwal wrote:
> 
> Pedro:
>> > Did this kernel port make it upstream without PTRACE_GETREGSET?
>> > If there's support for that, can you please switch to using it?
>>> >>Please answer all questions.
> Sure.  The Kernel code(ptrace.h) for Microblaze doesn't have upstream code without PTRACE_GETREGSET.
> 
> Pedro:
>> > PTRACE_GETREGS is supposed to an old way of doing things...
>>> >>And address all comments.
> The Microblaze Kernel code PTRACE_GETREGS is always defined and there is no conditional compilation which is without the PTRACE_GETREGS. So I agree with Pedro comment of not using #ifdef PTRACE_GETREGS and in the patch submitted I have removed if #ifdef PTRACE_GETREGS which is not required.

>>PTRACE_GETREGSET != PTRACE_GETREGS

>>I'm asking for using the PTRACE_GETREGSET instead of PTRACE_GETREGS in new ports.  See background here:

  >>https://sourceware.org/ml/archer/2010-q3/msg00193.html

The changes are incorporated.

>> +#define microblaze_breakpoint_len 4
>>
>> Use CAPS for macros.

The changes are incorporated.

>>+  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
>>+
>>+  if (insn == microblaze_breakpoint)

>>Why use the explicit length rather than the macro you just defined?
>>Why not use sizeof (insn)?

The changes are incorporated.

The below patch incorporated all the above feedbacks.

[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-18  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.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.

                === gdb Summary ===

# of expected passes            7804
# of unexpected failures        2263
# of unexpected successes       2
# of expected failures          5
# of known failures             22
# of unresolved testcases       29
# of untested testcases         41
# of unsupported tests          125

Thanks & Regards
Ajit

Thanks,
Pedro Alves


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

From 188b4f2a25240e4b4d21bf61217c7ee03b051030 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Sat, 18 Oct 2014 00:34:39 +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-18  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 |  237 ++++++++++++++++++++++++++++++++++
 4 files changed, 248 insertions(+), 0 deletions(-)
 create mode 100644 gdb/gdbserver/linux-microblaze-low.c

diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index d362cd9..fc2ad5c 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -344,6 +344,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..9488b25
--- /dev/null
+++ b/gdb/gdbserver/linux-microblaze-low.c
@@ -0,0 +1,237 @@
+/* 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/ptrace.h>
+#include <sys/procfs.h>
+#include <linux/elf.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)
+#define MICROBLAZE_BREAKPOINT_LEN 4
+
+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;
+
+static int
+microblaze_breakpoint_at (CORE_ADDR where)
+{
+  unsigned long insn;
+
+  (*the_target->read_memory) (where, (unsigned char *) &insn,
+                              MICROBLAZE_BREAKPOINT_LEN);
+
+  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_GETREGSET, PTRACE_SETREGSET, NT_PRSTATUS,
+    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


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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-15 13:27     ` Pedro Alves
  2014-10-17 19:22       ` Ajit Kumar Agarwal
@ 2014-11-26 12:13       ` Ajit Kumar Agarwal
  2014-12-15 16:08         ` Ajit Kumar Agarwal
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-11-26 12:13 UTC (permalink / raw)
  To: Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

Hello Pedro:

Just wanted to check  if the changes were Okay and there weren't any issues in it. 
Please let me know if it's  good to commit.

Thanks & Regards
Ajit

-----Original Message-----
From: Ajit Kumar Agarwal 
Sent: Saturday, October 18, 2014 12:52 AM
To: 'Pedro Alves'; Michael Eager; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch] Microblaze: Port of Linux gdbserver



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com]
Sent: Wednesday, October 15, 2014 6:57 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 10/09/2014 07:54 PM, Ajit Kumar Agarwal wrote:
> 
> Pedro:
>> > Did this kernel port make it upstream without PTRACE_GETREGSET?
>> > If there's support for that, can you please switch to using it?
>>> >>Please answer all questions.
> Sure.  The Kernel code(ptrace.h) for Microblaze doesn't have upstream code without PTRACE_GETREGSET.
> 
> Pedro:
>> > PTRACE_GETREGS is supposed to an old way of doing things...
>>> >>And address all comments.
> The Microblaze Kernel code PTRACE_GETREGS is always defined and there is no conditional compilation which is without the PTRACE_GETREGS. So I agree with Pedro comment of not using #ifdef PTRACE_GETREGS and in the patch submitted I have removed if #ifdef PTRACE_GETREGS which is not required.

>>PTRACE_GETREGSET != PTRACE_GETREGS

>>I'm asking for using the PTRACE_GETREGSET instead of PTRACE_GETREGS in new ports.  See background here:

  >>https://sourceware.org/ml/archer/2010-q3/msg00193.html

The changes are incorporated.

>> +#define microblaze_breakpoint_len 4
>>
>> Use CAPS for macros.

The changes are incorporated.

>>+  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
>>+
>>+  if (insn == microblaze_breakpoint)

>>Why use the explicit length rather than the macro you just defined?
>>Why not use sizeof (insn)?

The changes are incorporated.

The below patch incorporated all the above feedbacks.

[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-18  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.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.

                === gdb Summary ===

# of expected passes            7804
# of unexpected failures        2263
# of unexpected successes       2
# of expected failures          5
# of known failures             22
# of unresolved testcases       29
# of untested testcases         41
# of unsupported tests          125

Thanks & Regards
Ajit

Thanks,
Pedro Alves


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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-11-26 12:13       ` Ajit Kumar Agarwal
@ 2014-12-15 16:08         ` Ajit Kumar Agarwal
  0 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-15 16:08 UTC (permalink / raw)
  To: Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

Hello Pedro:

In continuation to my last mail, just wanted to check if the changes were okay and there weren't any issues in it.
Please let me know if it's good to commit.

Thanks & Regards
Ajit

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Ajit Kumar Agarwal
Sent: Wednesday, November 26, 2014 5:43 PM
To: Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch] Microblaze: Port of Linux gdbserver

Hello Pedro:

Just wanted to check  if the changes were Okay and there weren't any issues in it. 
Please let me know if it's  good to commit.

Thanks & Regards
Ajit

-----Original Message-----
From: Ajit Kumar Agarwal 
Sent: Saturday, October 18, 2014 12:52 AM
To: 'Pedro Alves'; Michael Eager; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch] Microblaze: Port of Linux gdbserver



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com]
Sent: Wednesday, October 15, 2014 6:57 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 10/09/2014 07:54 PM, Ajit Kumar Agarwal wrote:
> 
> Pedro:
>> > Did this kernel port make it upstream without PTRACE_GETREGSET?
>> > If there's support for that, can you please switch to using it?
>>> >>Please answer all questions.
> Sure.  The Kernel code(ptrace.h) for Microblaze doesn't have upstream code without PTRACE_GETREGSET.
> 
> Pedro:
>> > PTRACE_GETREGS is supposed to an old way of doing things...
>>> >>And address all comments.
> The Microblaze Kernel code PTRACE_GETREGS is always defined and there is no conditional compilation which is without the PTRACE_GETREGS. So I agree with Pedro comment of not using #ifdef PTRACE_GETREGS and in the patch submitted I have removed if #ifdef PTRACE_GETREGS which is not required.

>>PTRACE_GETREGSET != PTRACE_GETREGS

>>I'm asking for using the PTRACE_GETREGSET instead of PTRACE_GETREGS in new ports.  See background here:

  >>https://sourceware.org/ml/archer/2010-q3/msg00193.html

The changes are incorporated.

>> +#define microblaze_breakpoint_len 4
>>
>> Use CAPS for macros.

The changes are incorporated.

>>+  (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
>>+
>>+  if (insn == microblaze_breakpoint)

>>Why use the explicit length rather than the macro you just defined?
>>Why not use sizeof (insn)?

The changes are incorporated.

The below patch incorporated all the above feedbacks.

[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-18  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.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.

                === gdb Summary ===

# of expected passes            7804
# of unexpected failures        2263
# of unexpected successes       2
# of expected failures          5
# of known failures             22
# of unresolved testcases       29
# of untested testcases         41
# of unsupported tests          125

Thanks & Regards
Ajit

Thanks,
Pedro Alves


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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-10-17 19:22       ` Ajit Kumar Agarwal
@ 2014-12-15 18:02         ` Pedro Alves
  2014-12-15 18:13           ` Michael Eager
  2014-12-18  8:57           ` Ajit Kumar Agarwal
  0 siblings, 2 replies; 59+ messages in thread
From: Pedro Alves @ 2014-12-15 18:02 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 10/17/2014 08:22 PM, Ajit Kumar Agarwal wrote:

> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
> 
>                 === gdb Summary ===
> 
> # of expected passes            7804
> # of unexpected failures        2263

Over 2000 unexpected failures is not very reassuring though.
Have you looked at the logs to get an idea of what might be broken?


> +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

Please name give the .c file the same base name as the .dat file -> microblaze-with-stack-protect.c .

> +/* Provide only a fill function for the general register set.

I don't understand this comment.  You have a store function just below?

> 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_GETREGSET, PTRACE_SETREGSET, NT_PRSTATUS,
> +    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 }

Can't see why you'd need two "null" entries?

Thanks,
Pedro Alves

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-15 18:02         ` Pedro Alves
@ 2014-12-15 18:13           ` Michael Eager
  2014-12-18  8:58             ` Ajit Kumar Agarwal
  2014-12-18  8:57           ` Ajit Kumar Agarwal
  1 sibling, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-12-15 18:13 UTC (permalink / raw)
  To: Pedro Alves, Ajit Kumar Agarwal, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 12/15/14 10:02, Pedro Alves wrote:
> On 10/17/2014 08:22 PM, Ajit Kumar Agarwal wrote:
>
>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>
>>                  === gdb Summary ===
>>
>> # of expected passes            7804
>> # of unexpected failures        2263
>
> Over 2000 unexpected failures is not very reassuring though.
> Have you looked at the logs to get an idea of what might be broken?

Bare metal test results show ~350 failures.  The gdbserver results
should be similar.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-15 18:02         ` Pedro Alves
  2014-12-15 18:13           ` Michael Eager
@ 2014-12-18  8:57           ` Ajit Kumar Agarwal
  2014-12-18 11:28             ` Pedro Alves
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-18  8:57 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Monday, December 15, 2014 11:33 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 10/17/2014 08:22 PM, Ajit Kumar Agarwal wrote:

> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
> 
>                 === gdb Summary ===
> 
> # of expected passes            7804
> # of unexpected failures        2263

>>Over 2000 unexpected failures is not very reassuring though.
>>Have you looked at the logs to get an idea of what might be broken?

We have looked at the log files for the failures. Here are the main categories of the failure.

1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.
2.  Failures for signals is around 357.
3. Watch point  failures are around 817.

Main total categories of the failure = 376 + 357 + 817 =  1550.

These failures are not because of  gdbserver patch and they seem to exist prior to this patch. 

> +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

>>Please name give the .c file the same base name as the .dat file -> microblaze-with-stack-protect.c .

I will incorporate this.


> +/* Provide only a fill function for the general register set.

>>I don't understand this comment.  You have a store function just below?

Sorry. We meant by the comment that the below functions are used for fill_function and store_function.

> 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_GETREGSET, PTRACE_SETREGSET, NT_PRSTATUS,
> +    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 }

>>Can't see why you'd need two "null" entries?

Thanks. Only one  entry is sufficient.

Thanks & Regards
Ajit

Thanks,
Pedro Alves


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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-15 18:13           ` Michael Eager
@ 2014-12-18  8:58             ` Ajit Kumar Agarwal
  2014-12-18 16:10               ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-18  8:58 UTC (permalink / raw)
  To: Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Monday, December 15, 2014 11:44 PM
To: Pedro Alves; Ajit Kumar Agarwal; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver

On 12/15/14 10:02, Pedro Alves wrote:
> On 10/17/2014 08:22 PM, Ajit Kumar Agarwal wrote:
>
>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>
>>                  === gdb Summary ===
>>
>> # of expected passes            7804
>> # of unexpected failures        2263
>
> Over 2000 unexpected failures is not very reassuring though.
> Have you looked at the logs to get an idea of what might be broken?

>>Bare metal test results show ~350 failures.  The gdbserver results should be similar.

push_dummy_code feature is not implemented in Microblaze port.
The total failure due to this functionality is 592 in bare metal. Of this  gdb.base failures are 357.

May we know what you meant by ~350 failures.
-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-18  8:57           ` Ajit Kumar Agarwal
@ 2014-12-18 11:28             ` Pedro Alves
  2014-12-18 16:53               ` Ajit Kumar Agarwal
  2014-12-19 10:26               ` Ajit Kumar Agarwal
  0 siblings, 2 replies; 59+ messages in thread
From: Pedro Alves @ 2014-12-18 11:28 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 12/18/2014 08:56 AM, Ajit Kumar Agarwal wrote:
> From: Pedro Alves [mailto:palves@redhat.com] 
> On 10/17/2014 08:22 PM, Ajit Kumar Agarwal wrote:
> 
>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>
>>                 === gdb Summary ===
>>
>> # of expected passes            7804
>> # of unexpected failures        2263
> 
>>> Over 2000 unexpected failures is not very reassuring though.
>>> Have you looked at the logs to get an idea of what might be broken?
> 
> We have looked at the log files for the failures. Here are the main categories of the failure.
> 
> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.

Eh, no inferior function call support.  Are you planning on
implementing this?

You can set gdb,cannot_call_functions in your board file to
skip the affected tests meanwhile.

> 2.  Failures for signals is around 357.

What sort of failures?

> 3. Watch point  failures are around 817.

Set gdb,no_hardware_watchpoints in the board file.

> 
> Main total categories of the failure = 376 + 357 + 817 =  1550.
> 
> These failures are not because of  gdbserver patch and they seem to exist prior to this patch. 

Thanks,
Pedro Alves

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-18  8:58             ` Ajit Kumar Agarwal
@ 2014-12-18 16:10               ` Michael Eager
  0 siblings, 0 replies; 59+ messages in thread
From: Michael Eager @ 2014-12-18 16:10 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 12/18/14 00:58, Ajit Kumar Agarwal wrote:
>
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagercon.com]
> Sent: Monday, December 15, 2014 11:44 PM
> To: Pedro Alves; Ajit Kumar Agarwal; Joel Brobecker
> Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch] Microblaze: Port of Linux gdbserver
>
> On 12/15/14 10:02, Pedro Alves wrote:
>> On 10/17/2014 08:22 PM, Ajit Kumar Agarwal wrote:
>>
>>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>>
>>>                   === gdb Summary ===
>>>
>>> # of expected passes            7804
>>> # of unexpected failures        2263
>>
>> Over 2000 unexpected failures is not very reassuring though.
>> Have you looked at the logs to get an idea of what might be broken?
>
>>> Bare metal test results show ~350 failures.  The gdbserver results should be similar.
>
> push_dummy_code feature is not implemented in Microblaze port.
> The total failure due to this functionality is 592 in bare metal. Of this  gdb.base failures are 357.
>
> May we know what you meant by ~350 failures.

Sorry, I was looking at GCC failures.

Running gdb regression test on bare metal gives the following:

# of expected passes		6481
# of unexpected failures	1053
# of expected failures		6
# of known failures		21
# of unresolved testcases	12
# of untested testcases	31
# of unsupported tests		21

That's 1053 failures, compared to the 2263 using gdbserver.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-18 11:28             ` Pedro Alves
@ 2014-12-18 16:53               ` Ajit Kumar Agarwal
  2014-12-18 17:40                 ` Pedro Alves
  2014-12-19 10:26               ` Ajit Kumar Agarwal
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-18 16:53 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Thursday, December 18, 2014 4:58 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 12/18/2014 08:56 AM, Ajit Kumar Agarwal wrote:
> From: Pedro Alves [mailto:palves@redhat.com] On 10/17/2014 08:22 PM, 
> Ajit Kumar Agarwal wrote:
> 
>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>
>>                 === gdb Summary ===
>>
>> # of expected passes            7804
>> # of unexpected failures        2263
> 
>>> Over 2000 unexpected failures is not very reassuring though.
>>> Have you looked at the logs to get an idea of what might be broken?
> 
> We have looked at the log files for the failures. Here are the main categories of the failure.
> 
> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.

>>Eh, no inferior function call support.  Are you planning on implementing this?

Currently in the gdb  microblaze-tdep.c, the following code is there for push_dummy_code.

static CORE_ADDR
microblaze_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
                            CORE_ADDR funcaddr,
                            struct value **args, int nargs,
                            struct type *value_type,
                            CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
                            struct regcache *regcache)
{
  error (_("push_dummy_code not implemented"));
  return sp;
}
This causes the failures.

>>You can set gdb,cannot_call_functions in your board file to skip the affected tests meanwhile.

Thanks. I will use this option.

> 2.  Failures for signals is around 357.

>>What sort of failures?

We are investigating the failures caused due to signal. Would let you know.

> 3. Watch point  failures are around 817.

>>Set gdb,no_hardware_watchpoints in the board file.

Thanks. I will use this option.
> 
> Main total categories of the failure = 376 + 357 + 817 =  1550.
> 
> These failures are not because of  gdbserver patch and they seem to exist prior to this patch. 

Thanks & Regards
Ajit

Thanks,
Pedro Alves


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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-18 16:53               ` Ajit Kumar Agarwal
@ 2014-12-18 17:40                 ` Pedro Alves
  2014-12-19  8:27                   ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2014-12-18 17:40 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 12/18/2014 04:53 PM, Ajit Kumar Agarwal wrote:

>> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.
> 
>>> Eh, no inferior function call support.  Are you planning on implementing this?
> 
> Currently in the gdb  microblaze-tdep.c, the following code is there for push_dummy_code.
> 
> static CORE_ADDR
> microblaze_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
>                             CORE_ADDR funcaddr,
>                             struct value **args, int nargs,
>                             struct type *value_type,
>                             CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
>                             struct regcache *regcache)
> {
>   error (_("push_dummy_code not implemented"));
>   return sp;
> }
> This causes the failures.

Yes, this are the hooks for supporting calling inferior functions
from gdb: "print foo()", etc.

The question was whether you were planning on replacing the
errors (this and microblaze_push_dummy_call's) with a real
implementation.  :-)

Thanks,
Pedro Alves

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-18 17:40                 ` Pedro Alves
@ 2014-12-19  8:27                   ` Ajit Kumar Agarwal
  2014-12-19 10:56                     ` Pedro Alves
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-19  8:27 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Thursday, December 18, 2014 11:10 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 12/18/2014 04:53 PM, Ajit Kumar Agarwal wrote:

>> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.
> 
>>> Eh, no inferior function call support.  Are you planning on implementing this?
> 
> Currently in the gdb  microblaze-tdep.c, the following code is there for push_dummy_code.
> 
> static CORE_ADDR
> microblaze_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
>                             CORE_ADDR funcaddr,
>                             struct value **args, int nargs,
>                             struct type *value_type,
>                             CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
>                             struct regcache *regcache) {
>   error (_("push_dummy_code not implemented"));
>   return sp;
> }
> This causes the failures.

>>Yes, this are the hooks for supporting calling inferior functions from gdb: "print foo()", etc.

>>The question was whether you were planning on replacing the errors (this and microblaze_push_dummy_call's) with a real implementation.  :-)

Is this functionality optional as I can see many targets doesn't have the implementation for the inferior call function support. We do have plans for the implementation but I am wondering why this functionality is not implemented for many other targets like ARM.

Thanks & Regards
Ajit

Thanks,
Pedro Alves


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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-18 11:28             ` Pedro Alves
  2014-12-18 16:53               ` Ajit Kumar Agarwal
@ 2014-12-19 10:26               ` Ajit Kumar Agarwal
  2014-12-19 11:02                 ` Pedro Alves
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-19 10:26 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Thursday, December 18, 2014 4:58 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 12/18/2014 08:56 AM, Ajit Kumar Agarwal wrote:
> From: Pedro Alves [mailto:palves@redhat.com] On 10/17/2014 08:22 PM, 
> Ajit Kumar Agarwal wrote:
> 
>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>
>>                 === gdb Summary ===
>>
>> # of expected passes            7804
>> # of unexpected failures        2263
> 
>>> Over 2000 unexpected failures is not very reassuring though.
>>> Have you looked at the logs to get an idea of what might be broken?
> 
> We have looked at the log files for the failures. Here are the main categories of the failure.
> 
> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.

>>Eh, no inferior function call support.  Are you planning on implementing this?

>>You can set gdb,cannot_call_functions in your board file to skip the affected tests meanwhile.

> 2.  Failures for signals is around 357.

>>What sort of failures?

> 3. Watch point  failures are around 817.

>>Set gdb,no_hardware_watchpoints in the board file.

Thanks. We have used the following gdb options as per your suggestions.

set_board_info gdb,no_hardware_watchpoints 1 set_board_info gdb,cannot_call_functions 1 set_board_info gdb,nosignals 1

The gdb summary for gdb.base is as follows:

                === gdb Summary ===

# of expected passes            6047
# of unexpected failures        539
# of expected failures          17
# of known failures             21
# of unresolved testcases       26
# of untested testcases         43
# of unsupported tests          133

I will send the modified patch incorporating your comments.

Thanks & Regards
Ajit


> 
> Main total categories of the failure = 376 + 357 + 817 =  1550.
> 
> These failures are not because of  gdbserver patch and they seem to exist prior to this patch. 

Thanks,
Pedro Alves


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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-19  8:27                   ` Ajit Kumar Agarwal
@ 2014-12-19 10:56                     ` Pedro Alves
  0 siblings, 0 replies; 59+ messages in thread
From: Pedro Alves @ 2014-12-19 10:56 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 12/19/2014 08:27 AM, Ajit Kumar Agarwal wrote:

>>> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.
>>
>>>> Eh, no inferior function call support.  Are you planning on implementing this?
>>
>> Currently in the gdb  microblaze-tdep.c, the following code is there for push_dummy_code.
>>
>> static CORE_ADDR
>> microblaze_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
>>                             CORE_ADDR funcaddr,
>>                             struct value **args, int nargs,
>>                             struct type *value_type,
>>                             CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
>>                             struct regcache *regcache) {
>>   error (_("push_dummy_code not implemented"));
>>   return sp;
>> }
>> This causes the failures.
> 
>>> Yes, this are the hooks for supporting calling inferior functions from gdb: "print foo()", etc.
> 
>>> The question was whether you were planning on replacing the errors (this and microblaze_push_dummy_call's) with a real implementation.  :-)
> 
> Is this functionality optional as I can see many targets doesn't have the implementation for the inferior call function support. We do have plans for the implementation but I am wondering why this functionality is not implemented for many other targets like ARM.

The real meat is in gdbarch_push_dummy_call, which is throws an
error on microblaze too (it's just below microblaze_push_dummy_code).

gdbarch_push_dummy_code is optional because it's only used if
gdbarch_call_dummy_location is ON_STACK.

Inferior function call functionality is definitely implemented on ARM (and
on most if not all, other ports).  See arm_push_dummy_call.

Thanks,
Pedro Alves

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

* Re: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-19 10:26               ` Ajit Kumar Agarwal
@ 2014-12-19 11:02                 ` Pedro Alves
  2014-12-19 18:06                   ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2014-12-19 11:02 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 12/19/2014 10:26 AM, Ajit Kumar Agarwal wrote:
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com] 
> Sent: Thursday, December 18, 2014 4:58 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 12/18/2014 08:56 AM, Ajit Kumar Agarwal wrote:
>> From: Pedro Alves [mailto:palves@redhat.com] On 10/17/2014 08:22 PM, 
>> Ajit Kumar Agarwal wrote:
>>
>>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>>
>>>                 === gdb Summary ===
>>>
>>> # of expected passes            7804
>>> # of unexpected failures        2263
>>
>>>> Over 2000 unexpected failures is not very reassuring though.
>>>> Have you looked at the logs to get an idea of what might be broken?
>>
>> We have looked at the log files for the failures. Here are the main categories of the failure.
>>
>> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.
> 
>>> Eh, no inferior function call support.  Are you planning on implementing this?
> 
>>> You can set gdb,cannot_call_functions in your board file to skip the affected tests meanwhile.
> 
>> 2.  Failures for signals is around 357.
> 
>>> What sort of failures?
> 
>> 3. Watch point  failures are around 817.
> 
>>> Set gdb,no_hardware_watchpoints in the board file.
> 
> Thanks. We have used the following gdb options as per your suggestions.
> 
> set_board_info gdb,no_hardware_watchpoints 1 set_board_info gdb,cannot_call_functions 1 set_board_info gdb,nosignals 1
> 

To be clear, gdb,nosignals is for targets that truly have no concept
of signals.  A Linux port should not need that...  It's probably
masking out real problems.

> The gdb summary for gdb.base is as follows:
> 
>                 === gdb Summary ===
> 
> # of expected passes            6047
> # of unexpected failures        539

FYI, this is way higher than I'd expect after disabling all
that functionality.

> # of expected failures          17
> # of known failures             21
> # of unresolved testcases       26
> # of untested testcases         43
> # of unsupported tests          133
> 
> I will send the modified patch incorporating your comments.

Thanks.
Pedro Alves

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

* RE: [Patch] Microblaze: Port of Linux gdbserver
  2014-12-19 11:02                 ` Pedro Alves
@ 2014-12-19 18:06                   ` Ajit Kumar Agarwal
  0 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-12-19 18:06 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Pedro Alves
Sent: Friday, December 19, 2014 4:33 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 12/19/2014 10:26 AM, Ajit Kumar Agarwal wrote:
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Thursday, December 18, 2014 4:58 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 12/18/2014 08:56 AM, Ajit Kumar Agarwal wrote:
>> From: Pedro Alves [mailto:palves@redhat.com] On 10/17/2014 08:22 PM, 
>> Ajit Kumar Agarwal wrote:
>>
>>> Gdb.base gdb testsuite is run and here is the status of gdb testsuite run for gdb.base.
>>>
>>>                 === gdb Summary ===
>>>
>>> # of expected passes            7804
>>> # of unexpected failures        2263
>>
>>>> Over 2000 unexpected failures is not very reassuring though.
>>>> Have you looked at the logs to get an idea of what might be broken?
>>
>> We have looked at the log files for the failures. Here are the main categories of the failure.
>>
>> 1. push_dummy_code is not implemented for Micro blaze port  due to this  there are 350+ failures.
> 
>>> Eh, no inferior function call support.  Are you planning on implementing this?
> 
>>> You can set gdb,cannot_call_functions in your board file to skip the affected tests meanwhile.
> 
>> 2.  Failures for signals is around 357.
> 
>>> What sort of failures?
> 
>> 3. Watch point  failures are around 817.
> 
>>> Set gdb,no_hardware_watchpoints in the board file.
> 
> Thanks. We have used the following gdb options as per your suggestions.
> 
> set_board_info gdb,no_hardware_watchpoints 1 set_board_info 
> gdb,cannot_call_functions 1 set_board_info gdb,nosignals 1
> 

>>To be clear, gdb,nosignals is for targets that truly have no concept of signals.  A Linux port should not need that...  It's probably masking out real problems.

Thanks. I have added gdb,nosignals to investigate the failures for signal handling. Sorry for that.

> The gdb summary for gdb.base is as follows:
> 
>                 === gdb Summary ===
> 
> # of expected passes            6047
> # of unexpected failures        539

>>FYI, this is way higher than I'd expect after disabling all that functionality.


Could you please let me know what is the expected failures  after disabling all that functionality.

Thanks & Regards
Ajit

> # of expected failures          17
> # of known failures             21
> # of unresolved testcases       26
> # of untested testcases         43
> # of unsupported tests          133
> 
> I will send the modified patch incorporating your comments.

Thanks.
Pedro Alves


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

* RE: [Patch] Microblaze: Port of Linux gdbserver
@ 2014-10-08 14:59 Ajit Kumar Agarwal
  0 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-10-08 14:59 UTC (permalink / raw)
  To: Pedro Alves, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

With the below gdbserver Patch, here are the test results of running the gdb regression testsuite (gdb.base) . This tests results with the following patches already sent to FSF community for review.

GDB Client:

1. Add Little Endian breakpoint.
2. Add Software Single Step.

With the above GDB Client, below gdbserver patches  are used.


                === gdb Summary ===

# of expected passes            7680
# of unexpected failures        2138
# of unexpected successes       2
# of expected failures          5
# of known failures             22
# of unresolved testcases       28
# of untested testcases         42
# of unsupported tests          124

Thanks & Regards
Ajit
-----Original Message-----
From: Ajit Kumar Agarwal 
Sent: Wednesday, October 08, 2014 7:22 PM
To: 'Pedro Alves'; Michael Eager; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: [Patch] Microblaze: Port of Linux gdbserver

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

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-30 13:37                           ` Pedro Alves
@ 2014-09-30 14:21                             ` Ajit Kumar Agarwal
  0 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-30 14:21 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

Hello Pedro:

We have holidays in India till Monday. I will send the updated and new patches by Tuesday( 7th Oct 2014).

Thanks & Regards
Ajit

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Tuesday, September 30, 2014 7:07 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/30/2014 02:27 PM, Ajit Kumar Agarwal wrote:
> 
> 
>> > 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
> I have created this file instead of editing. I should have written as  New file in the ChangeLog. Still You want me to send a separate patch.

??  No, you haven't created it.  It's already in the tree.

Please send a separate patch that edits gdb/features/Makefile, where it reads:

 microblaze-expedite = r1,pc

and that regenerates the .dat file with that Makefile.

> 
> Thanks  & Regards
> Ajit
> 
>> >  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

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-30 13:27                         ` Ajit Kumar Agarwal
@ 2014-09-30 13:37                           ` Pedro Alves
  2014-09-30 14:21                             ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2014-09-30 13:37 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/30/2014 02:27 PM, Ajit Kumar Agarwal wrote:
> 
> 
>> > 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
> I have created this file instead of editing. I should have written as  New file in the ChangeLog. Still You want me to send a separate patch.

??  No, you haven't created it.  It's already in the tree.

Please send a separate patch that edits gdb/features/Makefile, where
it reads:

 microblaze-expedite = r1,pc

and that regenerates the .dat file with that Makefile.

> 
> Thanks  & Regards
> Ajit
> 
>> >  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

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-30 11:43                       ` Pedro Alves
@ 2014-09-30 13:27                         ` Ajit Kumar Agarwal
  2014-09-30 13:37                           ` Pedro Alves
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-30 13:27 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



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

I will do this and send a separate independent patch.

> +#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

I have created this file instead of editing. I should have written as  New file in the ChangeLog. Still You want me to send a separate patch.

Thanks  & Regards
Ajit

>  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

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-23 12:49                     ` Ajit Kumar Agarwal
@ 2014-09-30 11:43                       ` Pedro Alves
  2014-09-30 13:27                         ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2014-09-30 11:43 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

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

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-17  8:15                   ` Pedro Alves
  2014-09-17  8:20                     ` Ajit Kumar Agarwal
@ 2014-09-23 12:49                     ` Ajit Kumar Agarwal
  2014-09-30 11:43                       ` Pedro Alves
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-23 12:49 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

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

Hello Pedro:

Please find the updated patch with below feedbacks incorporated.

Author: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date:   Tue Sep 23 18:08:46 2014 +0530

    [Patch, microblaze]: Port of Linux gdbserver
    
    This patch is the port of Linux gdbserver.
    
    gdb/ChangeLog:
    2014-09-23  Ajit Agarwal  <ajitkum@xilinx.com>
    
        * configure.tgt (build_gdbserver): New Definition.
        * regformats/microblaze-with-stack-protect.dat (expedite):
        Update the expedite as rpc.
    
    gdb/gdbserver/ChangeLog:
    
        * 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: Ajit Kumar Agarwal 
Sent: Wednesday, September 17, 2014 1:50 PM
To: 'Pedro Alves'; Michael Eager; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch, microblaze]: Port of Linux gdbserver



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com]
Sent: Wednesday, September 17, 2014 1:45 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/17/2014 07:16 AM, Ajit Kumar Agarwal wrote:

> From: Pedro Alves [mailto:palves@redhat.com] On 09/16/2014 07:41 AM, 
> Ajit Kumar Agarwal wrote:
>> This is needed as gdbserver code expects the register pc as "pc" instead of "rpc" for baremetel. The microblaze-linux-core.xml is changed from "rpc" to "pc" for gdbserver code to work.
> 
>>> This doesn't make much sense to me.  Can you expand please?  Why would you want the register to be named differently on Linux?  We've defined the >>org.gnu.gdb.microblaze.core with "rpc", presumably because that's what the architecture calls that core register.
> 
>>> Not reporting all the registers with the exact names GDB reports should be making GDB consider the description invalid.  Aren't you seeing that happen?
> 
> In Microblaze gdbserver code linux-microblaze-low.c we have are passing the "pc" in supply_register_by_name and the baremetal org.gnu.gdb.microblaze.core its been defined as "rpc". Due to this in regcache.c where the find_regno function compares "pc" passed with "rpc" and reports failures. That is why we have create microblaze-linux-core.xml to have "pc" instead of "rpc".
> 
> static void
> microblaze_set_pc (struct regcache *regcache, CORE_ADDR pc) {
>   unsigned long newpc = pc;
>   supply_register_by_name (regcache, "pc", &newpc); }

>>But that is port-specific code that you're adding with this patch.
>>So just write instead:

  >>supply_register_by_name (regcache, "rpc", &newpc);

>>Why wouldn't that work?  But maybe I'm missing something.

Thanks Pedro !! I will make the change from "pc" to "rpc" in supply_register_by_name.  

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

Pedro Alves


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

From ede7eb935c766bf60aab649f01bbead1db35d470 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Tue, 23 Sep 2014 18:08:46 +0530
Subject: [PATCH] [Patch, microblaze]: Port of Linux gdbserver

This patch is the port of Linux gdbserver.

gdb/ChangeLog:
2014-09-23  Ajit Agarwal  <ajitkum@xilinx.com>

	* configure.tgt (build_gdbserver): New Definition.
	* regformats/microblaze-with-stack-protect.dat (expedite):
	Update the expedite as rpc.

gdb/gdbserver/ChangeLog:

	* 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 ++++++++++++++++++++++
 gdb/regformats/microblaze-with-stack-protect.dat |    2 +-
 5 files changed, 246 insertions(+), 1 deletions(-)
 create mode 100644 gdb/gdbserver/linux-microblaze-low.c

diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 01311b2..d3381a0 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 e9d6b15..7d22e72 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 \
@@ -365,6 +366,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
@@ -635,6 +637,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..6eadd4f 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -194,6 +194,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
+			;;
   nios2*-*-linux*)	srv_regobj="nios2-linux.o"
 			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
 			srv_xmlfiles="nios2-linux.xml"
diff --git a/gdb/gdbserver/linux-microblaze-low.c b/gdb/gdbserver/linux-microblaze-low.c
new file mode 100644
index 0000000..199d8c7
--- /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	\
+  (sizeof microblaze_regmap / sizeof microblaze_regmap[0])
+
+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;
+}
+
+#ifdef HAVE_PTRACE_GETREGS
+
+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]);
+}
+
+#endif /* HAVE_PTRACE_GETREGS */
+
+static struct regset_info microblaze_regsets[] = {
+#ifdef HAVE_PTRACE_GETREGS
+  { 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 }
+};
+
+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);
+}
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
 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


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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-17  9:36                 ` Ajit Kumar Agarwal
@ 2014-09-17 14:12                   ` Michael Eager
  0 siblings, 0 replies; 59+ messages in thread
From: Michael Eager @ 2014-09-17 14:12 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/17/14 02:36, Ajit Kumar Agarwal wrote:
> As the current testcase we sent is not reproducing  the issue at your end, we think the current gdb testsuite failures are the best examples to reproduce the problem.
> Along with these three patches and the gdbserver patch, the gdb testsuites will provide the results expected.

No, that simply is unsatisfactory.

A bad test case for a patch is not fixed by a vague
statement that some other set of patches fixes some
other unstated test case failures.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-16 12:06               ` Michael Eager
@ 2014-09-17  9:36                 ` Ajit Kumar Agarwal
  2014-09-17 14:12                   ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-17  9:36 UTC (permalink / raw)
  To: Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Tuesday, September 16, 2014 5:36 PM
To: Ajit Kumar Agarwal; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

On 09/15/14 23:41, Ajit Kumar Agarwal wrote:
>
>>> What are the results of running the gdb regression test suite using gdbserver on the target after applying this patch?
>
> This patch requires the below patches which are already sent to FSF to be applied for the expected results with gdb regression testsuite.
> The details and the related reasons are given below.

>>If a patch has prerequisites, please identify them.

Along with the  two patches given below the patch for linux_remove_breakpoint support are the prerequisites. 

> [Patch, microblaze] Add little-endian breakpoint.
> [Patch, microblaze]: Add support of microblaze software single 
> stepping
>
> The above patches were already sent to FSF and are yet to be applied.

>>Please respond to comments on the first patch.

As suggested the response is sent over the first patch.

>>For second patch, the test case you provided does not show the failure.

As the current testcase we sent is not reproducing  the issue at your end, we think the current gdb testsuite failures are the best examples to reproduce the problem.
Along with these three patches and the gdbserver patch, the gdb testsuites will provide the results expected.

> [Patch,microblaze]: Add support linux_memory_remove_breakpoints.
>
> As suggested by Joel  to not to  mix the  gdbserver patch with gdb patch, I will be sending the patch "Add Support linux_memory_remove_breakpoints" separately.

>>I suggest that you fix problems with the previous patches before submitting more.

Ok.

>
> With the above mentioned gdb patches  and the gdbserver patch,  below are the results. All the executables  are little endian binaries compiled with microblazeel-xilinux-linux-gnu.
>
>                  === gdb Summary ===
>
> # of expected passes            8146
> # of unexpected failures        2470
> # of unexpected successes       2
> # of expected failures          6
> # of known failures             21
> # of unresolved testcases       28
> # of untested testcases         44
> # of unsupported tests          125

>>Thanks.  What is your testing environment?

Xilinux Petalinux Kernel is booted on a KC705 design.  gdb testsuite is using the  Linux toolchain (compiled with microblazeel-xilinx-linux-gnu(little-endian)). 

Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-17  8:15                   ` Pedro Alves
@ 2014-09-17  8:20                     ` Ajit Kumar Agarwal
  2014-09-23 12:49                     ` Ajit Kumar Agarwal
  1 sibling, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-17  8:20 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Wednesday, September 17, 2014 1:45 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/17/2014 07:16 AM, Ajit Kumar Agarwal wrote:

> From: Pedro Alves [mailto:palves@redhat.com] On 09/16/2014 07:41 AM, 
> Ajit Kumar Agarwal wrote:
>> This is needed as gdbserver code expects the register pc as "pc" instead of "rpc" for baremetel. The microblaze-linux-core.xml is changed from "rpc" to "pc" for gdbserver code to work.
> 
>>> This doesn't make much sense to me.  Can you expand please?  Why would you want the register to be named differently on Linux?  We've defined the >>org.gnu.gdb.microblaze.core with "rpc", presumably because that's what the architecture calls that core register.
> 
>>> Not reporting all the registers with the exact names GDB reports should be making GDB consider the description invalid.  Aren't you seeing that happen?
> 
> In Microblaze gdbserver code linux-microblaze-low.c we have are passing the "pc" in supply_register_by_name and the baremetal org.gnu.gdb.microblaze.core its been defined as "rpc". Due to this in regcache.c where the find_regno function compares "pc" passed with "rpc" and reports failures. That is why we have create microblaze-linux-core.xml to have "pc" instead of "rpc".
> 
> static void
> microblaze_set_pc (struct regcache *regcache, CORE_ADDR pc) {
>   unsigned long newpc = pc;
>   supply_register_by_name (regcache, "pc", &newpc); }

>>But that is port-specific code that you're adding with this patch.
>>So just write instead:

  >>supply_register_by_name (regcache, "rpc", &newpc);

>>Why wouldn't that work?  But maybe I'm missing something.

Thanks Pedro !! I will make the change from "pc" to "rpc" in supply_register_by_name.  

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

Pedro Alves


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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-17  6:16                 ` Ajit Kumar Agarwal
@ 2014-09-17  8:15                   ` Pedro Alves
  2014-09-17  8:20                     ` Ajit Kumar Agarwal
  2014-09-23 12:49                     ` Ajit Kumar Agarwal
  0 siblings, 2 replies; 59+ messages in thread
From: Pedro Alves @ 2014-09-17  8:15 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/17/2014 07:16 AM, Ajit Kumar Agarwal wrote:

> From: Pedro Alves [mailto:palves@redhat.com] 
> On 09/16/2014 07:41 AM, Ajit Kumar Agarwal wrote:
>> This is needed as gdbserver code expects the register pc as "pc" instead of "rpc" for baremetel. The microblaze-linux-core.xml is changed from "rpc" to "pc" for gdbserver code to work.
> 
>>> This doesn't make much sense to me.  Can you expand please?  Why would you want the register to be named differently on Linux?  We've defined the >>org.gnu.gdb.microblaze.core with "rpc", presumably because that's what the architecture calls that core register.
> 
>>> Not reporting all the registers with the exact names GDB reports should be making GDB consider the description invalid.  Aren't you seeing that happen?
> 
> In Microblaze gdbserver code linux-microblaze-low.c we have are passing the "pc" in supply_register_by_name and the baremetal org.gnu.gdb.microblaze.core its been defined as "rpc". Due to this in regcache.c where the find_regno function compares "pc" passed with "rpc" and reports failures. That is why we have create microblaze-linux-core.xml to have "pc" instead of "rpc".
> 
> static void
> microblaze_set_pc (struct regcache *regcache, CORE_ADDR pc)
> {
>   unsigned long newpc = pc;
>   supply_register_by_name (regcache, "pc", &newpc);
> }

But that is port-specific code that you're adding with this patch.
So just write instead:

  supply_register_by_name (regcache, "rpc", &newpc);

Why wouldn't that work?  But maybe I'm missing something.

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

Pedro Alves

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-16 17:04               ` Pedro Alves
@ 2014-09-17  6:16                 ` Ajit Kumar Agarwal
  2014-09-17  8:15                   ` Pedro Alves
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-17  6:16 UTC (permalink / raw)
  To: Pedro Alves, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

Hello Pedro:

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Tuesday, September 16, 2014 10:34 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/16/2014 07:41 AM, Ajit Kumar Agarwal wrote:
>>> >>This is identical to microblaze-with-stack-protect.c.  Both 
>>> >>specify tdesc_create_feature (result, "org.gnu.gdb.microblaze.core"); Why is this needed?
> This is needed as gdbserver code expects the register pc as "pc" instead of "rpc" for baremetel. The microblaze-linux-core.xml is changed from "rpc" to "pc" for gdbserver code to work.

>>This doesn't make much sense to me.  Can you expand please?  Why would you want the register to be named differently on Linux?  We've defined the >>org.gnu.gdb.microblaze.core with "rpc", presumably because that's what the architecture calls that core register.

>>Not reporting all the registers with the exact names GDB reports should be making GDB consider the description invalid.  Aren't you seeing that happen?

In Microblaze gdbserver code linux-microblaze-low.c we have are passing the "pc" in supply_register_by_name and the baremetal org.gnu.gdb.microblaze.core its been defined as "rpc". Due to this in regcache.c where the find_regno function compares "pc" passed with "rpc" and reports failures. That is why we have create microblaze-linux-core.xml to have "pc" instead of "rpc".

static void
microblaze_set_pc (struct regcache *regcache, CORE_ADDR pc)
{
  unsigned long newpc = pc;
  supply_register_by_name (regcache, "pc", &newpc);
}
/me looks at code

>>Oh, microblaze_gdbarch_init is incomplete...

  /* Check any target description for validity.  */
 >> if (tdesc_has_registers (tdesc))
    >>{
      >>const struct tdesc_feature *feature;
      >>int valid_p;
     >> int i;

      >>feature = tdesc_find_feature (tdesc,
           >>                         "org.gnu.gdb.microblaze.core");
      >>if (feature == NULL)
        >>return NULL;
      >>tdesc_data = tdesc_data_alloc ();

      >>valid_p = 1;
      >>for (i = 0; i < MICROBLAZE_NUM_CORE_REGS; i++)
       >> valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
            >>                                microblaze_register_names[i]);
      >>feature = tdesc_find_feature (tdesc,
           >>                         "org.gnu.gdb.microblaze.stack-protect");
     >> if (feature != NULL)
       >> {
         >> valid_p = 1;
          >>valid_p &= tdesc_numbered_register (feature, tdesc_data,
               >>                               MICROBLAZE_SLR_REGNUM,
                    >>                          "rslr");
          >>valid_p &= tdesc_numbered_register (feature, tdesc_data,
               >>                               MICROBLAZE_SHR_REGNUM,
                    >>                          "rshr");
        >>}
     >>}

>>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,
Pedro Alves

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-16  6:42             ` Ajit Kumar Agarwal
  2014-09-16 12:06               ` Michael Eager
@ 2014-09-16 17:04               ` Pedro Alves
  2014-09-17  6:16                 ` Ajit Kumar Agarwal
  1 sibling, 1 reply; 59+ messages in thread
From: Pedro Alves @ 2014-09-16 17:04 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/16/2014 07:41 AM, Ajit Kumar Agarwal wrote:
>>> >>This is identical to microblaze-with-stack-protect.c.  Both specify tdesc_create_feature (result, "org.gnu.gdb.microblaze.core");
>>> >>Why is this needed?
> This is needed as gdbserver code expects the register pc as "pc" instead of "rpc" for baremetel. The microblaze-linux-core.xml is changed from "rpc" to "pc" for gdbserver code to work.

This doesn't make much sense to me.  Can you expand please?  Why would
you want the register to be named differently on Linux?  We've
defined the org.gnu.gdb.microblaze.core with "rpc", presumably because
that's what the architecture calls that core register.

Not reporting all the registers with the exact names GDB reports
should be making GDB consider the description invalid.  Aren't you
seeing that happen?

/me looks at code

Oh, microblaze_gdbarch_init is incomplete...

  /* Check any target description for validity.  */
  if (tdesc_has_registers (tdesc))
    {
      const struct tdesc_feature *feature;
      int valid_p;
      int i;

      feature = tdesc_find_feature (tdesc,
                                    "org.gnu.gdb.microblaze.core");
      if (feature == NULL)
        return NULL;
      tdesc_data = tdesc_data_alloc ();

      valid_p = 1;
      for (i = 0; i < MICROBLAZE_NUM_CORE_REGS; i++)
        valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
                                            microblaze_register_names[i]);
      feature = tdesc_find_feature (tdesc,
                                    "org.gnu.gdb.microblaze.stack-protect");
      if (feature != NULL)
        {
          valid_p = 1;
          valid_p &= tdesc_numbered_register (feature, tdesc_data,
                                              MICROBLAZE_SLR_REGNUM,
                                              "rslr");
          valid_p &= tdesc_numbered_register (feature, tdesc_data,
                                              MICROBLAZE_SHR_REGNUM,
                                              "rshr");
        }
     }

Note nothing is done with valid_p.  It's write-only.  Compare with
other ports, like arm-tdep.c or mips-tdep.c.

Thanks,
Pedro Alves

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-16  6:42             ` Ajit Kumar Agarwal
@ 2014-09-16 12:06               ` Michael Eager
  2014-09-17  9:36                 ` Ajit Kumar Agarwal
  2014-09-16 17:04               ` Pedro Alves
  1 sibling, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-09-16 12:06 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/15/14 23:41, Ajit Kumar Agarwal wrote:
>
>>> What are the results of running the gdb regression test suite using gdbserver on the target after applying this patch?
>
> This patch requires the below patches which are already sent to FSF to be applied for the expected results with gdb regression testsuite.
> The details and the related reasons are given below.

If a patch has prerequisites, please identify them.

> [Patch, microblaze] Add little-endian breakpoint.
> [Patch, microblaze]: Add support of microblaze software single stepping
>
> The above patches were already sent to FSF and are yet to be applied.

Please respond to comments on the first patch.
For second patch, the test case you provided does not show the failure.

> [Patch,microblaze]: Add support linux_memory_remove_breakpoints.
>
> As suggested by Joel  to not to  mix the  gdbserver patch with gdb patch, I will be sending the patch "Add Support linux_memory_remove_breakpoints" separately.

I suggest that you fix problems with the previous patches before
submitting more.

>
> With the above mentioned gdb patches  and the gdbserver patch,  below are the results. All the executables  are little endian binaries compiled with microblazeel-xilinux-linux-gnu.
>
>                  === gdb Summary ===
>
> # of expected passes            8146
> # of unexpected failures        2470
> # of unexpected successes       2
> # of expected failures          6
> # of known failures             21
> # of unresolved testcases       28
> # of untested testcases         44
> # of unsupported tests          125

Thanks.  What is your testing environment?


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-12 15:38           ` Michael Eager
@ 2014-09-16  6:42             ` Ajit Kumar Agarwal
  2014-09-16 12:06               ` Michael Eager
  2014-09-16 17:04               ` Pedro Alves
  0 siblings, 2 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-16  6:42 UTC (permalink / raw)
  To: Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala



-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Friday, September 12, 2014 9:08 PM
To: Ajit Kumar Agarwal; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

On 09/12/14 01:38, Ajit Kumar Agarwal wrote:
> Forget to attach the Patch Resending it again.
>
> With feedback comments incorporated, Please find the updated patch.

Again, please do not top post.

Please edit your posts.  There is no need to include multiple copies of the ChangeLog.

>      [Patch, microblaze]: Port of Linux gdbserver
>
>      This patch is the port of Linux gdbserver.
>
>      gdb/ChangeLog:
>      2014-10-12  Ajit Agarwal  <ajitkum@xilinx.com>
>
>          * configure.tgt (build_gdbserver): New Definition.
>          * features/microblaze-linux-core.xml: New file.
>          * features/microblaze-linux-stack-protect.xml: New file.
>          * features/microblaze-linux-stack-protect.c: New file.

>>This is identical to microblaze-with-stack-protect.c.  Both specify tdesc_create_feature (result, "org.gnu.gdb.microblaze.core");

>>Why is this needed?

This is needed as gdbserver code expects the register pc as "pc" instead of "rpc" for baremetel. The microblaze-linux-core.xml is changed from "rpc" to "pc" for gdbserver code to work.

>
>      gdb/gdbserver/ChangeLog:
>
>          * 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

>>What are the results of running the gdb regression test suite using gdbserver on the target after applying this patch?

This patch requires the below patches which are already sent to FSF to be applied for the expected results with gdb regression testsuite. 
The details and the related reasons are given below.

[Patch, microblaze] Add little-endian breakpoint.
[Patch, microblaze]: Add support of microblaze software single stepping

The above patches were already sent to FSF and are yet to be applied.

[Patch,microblaze]: Add support linux_memory_remove_breakpoints.

As suggested by Joel  to not to  mix the  gdbserver patch with gdb patch, I will be sending the patch "Add Support linux_memory_remove_breakpoints" separately.

Without the above patches and running the gdb regression testsuite(gdb.base)  gives bad testsuite results.
Hence the gdb testsuite is stopped in between.

warning: Remote failure reply: E01
Ignoring packet error, continuing...
FAIL: gdb.base/annota1.exp: print array (timeout)

Due to this error the subsequent testcases fails and following results were obtained.

		=== gdb Summary ===

# of expected passes		67
# of unexpected failures	106
# of expected failures		1
# of unresolved testcases	6
# of untested testcases		4
# of unsupported tests		96

With the above mentioned gdb patches  and the gdbserver patch,  below are the results. All the executables  are little endian binaries compiled with microblazeel-xilinux-linux-gnu.

                === gdb Summary ===

# of expected passes            8146
# of unexpected failures        2470
# of unexpected successes       2
# of expected failures          6
# of known failures             21
# of unresolved testcases       28
# of untested testcases         44
# of unsupported tests          125

Thanks & Regards
Ajit
-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-12  8:39         ` Ajit Kumar Agarwal
@ 2014-09-12 15:38           ` Michael Eager
  2014-09-16  6:42             ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-09-12 15:38 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/12/14 01:38, Ajit Kumar Agarwal wrote:
> Forget to attach the Patch Resending it again.
>
> With feedback comments incorporated, Please find the updated patch.

Again, please do not top post.

Please edit your posts.  There is no need to include multiple copies
of the ChangeLog.

>      [Patch, microblaze]: Port of Linux gdbserver
>
>      This patch is the port of Linux gdbserver.
>
>      gdb/ChangeLog:
>      2014-10-12  Ajit Agarwal  <ajitkum@xilinx.com>
>
>          * configure.tgt (build_gdbserver): New Definition.
>          * features/microblaze-linux-core.xml: New file.
>          * features/microblaze-linux-stack-protect.xml: New file.
>          * features/microblaze-linux-stack-protect.c: New file.

This is identical to microblaze-with-stack-protect.c.  Both specify
tdesc_create_feature (result, "org.gnu.gdb.microblaze.core");

Why is this needed?


>
>      gdb/gdbserver/ChangeLog:
>
>          * 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

What are the results of running the gdb regression test suite
using gdbserver on the target after applying this patch?

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:43       ` Joel Brobecker
  2014-09-10 15:04         ` Ajit Kumar Agarwal
  2014-09-12  8:01         ` Ajit Kumar Agarwal
@ 2014-09-12  8:39         ` Ajit Kumar Agarwal
  2014-09-12 15:38           ` Michael Eager
  2 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-12  8:39 UTC (permalink / raw)
  To: Joel Brobecker, Michael Eager
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

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

Forget to attach the Patch Resending it again.

With feedback comments incorporated, Please find the updated patch.

    [Patch, microblaze]: Port of Linux gdbserver

    This patch is the port of Linux gdbserver.

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

        * configure.tgt (build_gdbserver): New Definition.
        * features/microblaze-linux-core.xml: New file.
        * features/microblaze-linux-stack-protect.xml: New file.
        * features/microblaze-linux-stack-protect.c: New file.

    gdb/gdbserver/ChangeLog:

        * 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: Ajit Kumar Agarwal 
Sent: Friday, September 12, 2014 1:31 PM
To: 'Joel Brobecker'; Michael Eager
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch, microblaze]: Port of Linux gdbserver

With feedback comments incorporated, Please find the updated patch.

    [Patch, microblaze]: Port of Linux gdbserver

    This patch is the port of Linux gdbserver.

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

        * configure.tgt (build_gdbserver): New Definition.
        * features/microblaze-linux-core.xml: New file.
        * features/microblaze-linux-stack-protect.xml: New file.
        * features/microblaze-linux-stack-protect.c: New file.

    gdb/gdbserver/ChangeLog:

        * 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: Joel Brobecker [mailto:brobecker@adacore.com]
Sent: Wednesday, September 10, 2014 8:13 PM
To: Ajit Kumar Agarwal
Cc: Michael Eager; gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

[with my patch-champion hat]

I can't review the gdbserver Changes, but I can look at the rest.

>     ChangeLog:
>     2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
> 
>         * configure.host (microblaze): New.
>         (microblaze*-*-linux*): New.
>         * configure.tgt (build_gdbserver): New Definition.
>         * gdbserver/Makefile.in (microblaze-linux.c): New target.
>         * gdbserver/configure.srv (microblaze*-*-linux*): New target.
>         * gdbserver/linux-microblaze-low.c: New file.

Sorry Ajit. I should have seen this in the previous iteration, but there are a couple more nits in the ChangeLog entry that you'll need to fix.

First, gdb and gdb/gdbserver have distinct ChangeLog files, so you'll need to have 2 ChangeLog entries if you touch both areas at the same time.  This means that the filenames in gdbserver should not be prefixed with "gdbserver/" in your ChangeLog entry.

Also, change "ChangeLog:" to "gdb/ChangeLog:" and "gdbserver/ChangeLog:".

Please exclude the configure.host change. This looks unrelated.

> diff --git a/gdb/configure.tgt b/gdb/configure.tgt index 
> 01311b2..e4894da 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
> 1447e61..b2a01f5 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -153,6 +153,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 \
> @@ -364,6 +365,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

There are indentation errors in the two hunks above. Use tabs.

> @@ -634,6 +636,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..16e44ee 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -194,6 +194,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
> +                        ;;
>    nios2*-*-linux*)	srv_regobj="nios2-linux.o"
>  			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
>  			srv_xmlfiles="nios2-linux.xml"
> diff --git a/gdb/gdbserver/linux-microblaze-low.c
> b/gdb/gdbserver/linux-microblaze-low.c
> new file mode 100644
> index 0000000..b86b4f5
> --- /dev/null
> +++ b/gdb/gdbserver/linux-microblaze-low.c

It looks to me like you have many many trailing spaces in the file you are submitting. Please strip them.

Some lines are too long, also.

--
Joel

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

From d4854eca16103ccfc52db9ddbfd78877a40a4187 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Fri, 12 Sep 2014 13:25:11 +0530
Subject: [PATCH] [Patch, microblaze]: Port of Linux gdbserver

This patch is the port of Linux gdbserver.

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

	* configure.tgt (build_gdbserver): New Definition.
	* features/microblaze-linux-core.xml: New file.
	* features/microblaze-linux-stack-protect.xml: New file.
	* features/microblaze-linux-stack-protect.c: New file.

gdb/gdbserver/ChangeLog:

	* 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/features/microblaze-linux-core.xml            |   67 ++++++
 gdb/features/microblaze-linux-stack-protect.c     |   79 +++++++
 gdb/features/microblaze-linux-stack-protect.xml   |   12 +
 gdb/gdbserver/Makefile.in                         |    4 +
 gdb/gdbserver/configure.srv                       |    6 +
 gdb/gdbserver/linux-microblaze-low.c              |  234 +++++++++++++++++++++
 gdb/regformats/microblaze-linux-stack-protect.dat |   63 ++++++
 8 files changed, 466 insertions(+), 0 deletions(-)
 create mode 100644 gdb/features/microblaze-linux-core.xml
 create mode 100644 gdb/features/microblaze-linux-stack-protect.c
 create mode 100644 gdb/features/microblaze-linux-stack-protect.xml
 create mode 100644 gdb/gdbserver/linux-microblaze-low.c
 create mode 100644 gdb/regformats/microblaze-linux-stack-protect.dat

diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 01311b2..d3381a0 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/features/microblaze-linux-core.xml b/gdb/features/microblaze-linux-core.xml
new file mode 100644
index 0000000..8e4fa01
--- /dev/null
+++ b/gdb/features/microblaze-linux-core.xml
@@ -0,0 +1,67 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2014 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.microblaze.core">
+  <reg name="r0" bitsize="32" regnum="0"/>
+  <reg name="r1" bitsize="32" type="data_ptr"/>
+  <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="r13" bitsize="32"/>
+  <reg name="r14" bitsize="32"/>
+  <reg name="r15" bitsize="32"/>
+  <reg name="r16" bitsize="32"/>
+  <reg name="r17" bitsize="32"/>
+  <reg name="r18" bitsize="32"/>
+  <reg name="r19" bitsize="32"/>
+  <reg name="r20" bitsize="32"/>
+  <reg name="r21" bitsize="32"/>
+  <reg name="r22" bitsize="32"/>
+  <reg name="r23" bitsize="32"/>
+  <reg name="r24" bitsize="32"/>
+  <reg name="r25" bitsize="32"/>
+  <reg name="r26" bitsize="32"/>
+  <reg name="r27" bitsize="32"/>
+  <reg name="r28" bitsize="32"/>
+  <reg name="r29" bitsize="32"/>
+  <reg name="r30" bitsize="32"/>
+  <reg name="r31" bitsize="32"/>
+  <reg name="pc" bitsize="32" type="code_ptr"/>
+  <reg name="rmsr" bitsize="32"/>
+  <reg name="rear" bitsize="32"/>
+  <reg name="resr" bitsize="32"/>
+  <reg name="rfsr" bitsize="32"/>
+  <reg name="rbtr" bitsize="32"/>
+  <reg name="rpvr0" bitsize="32"/>
+  <reg name="rpvr1" bitsize="32"/>
+  <reg name="rpvr2" bitsize="32"/>
+  <reg name="rpvr3" bitsize="32"/>
+  <reg name="rpvr4" bitsize="32"/>
+  <reg name="rpvr5" bitsize="32"/>
+  <reg name="rpvr6" bitsize="32"/>
+  <reg name="rpvr7" bitsize="32"/>
+  <reg name="rpvr8" bitsize="32"/>
+  <reg name="rpvr9" bitsize="32"/>
+  <reg name="rpvr10" bitsize="32"/>
+  <reg name="rpvr11" bitsize="32"/>
+  <reg name="redr" bitsize="32"/>
+  <reg name="rpid" bitsize="32"/>
+  <reg name="rzpr" bitsize="32"/>
+  <reg name="rtlbx" bitsize="32"/>
+  <reg name="rtlbsx" bitsize="32"/>
+  <reg name="rtlblo" bitsize="32"/>
+  <reg name="rtlbhi" bitsize="32"/>
+</feature>
diff --git a/gdb/features/microblaze-linux-stack-protect.c b/gdb/features/microblaze-linux-stack-protect.c
new file mode 100644
index 0000000..4b5eecf
--- /dev/null
+++ b/gdb/features/microblaze-linux-stack-protect.c
@@ -0,0 +1,79 @@
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: microblaze-linux-stack-protect.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_microblaze_linux_stack_protect;
+static void
+initialize_tdesc_microblaze_linux_stack_protect (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.microblaze.core");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "data_ptr");
+  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, "r13", 13, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r14", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r15", 15, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r16", 16, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r17", 17, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r18", 18, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r19", 19, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r20", 20, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r21", 21, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r22", 22, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r23", 23, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r24", 24, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r25", 25, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r26", 26, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r27", 27, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r28", 28, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r29", 29, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r30", 30, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r31", 31, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "pc", 32, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "rmsr", 33, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rear", 34, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "resr", 35, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rfsr", 36, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rbtr", 37, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr0", 38, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr1", 39, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr2", 40, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr3", 41, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr4", 42, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr5", 43, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr6", 44, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr7", 45, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr8", 46, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr9", 47, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr10", 48, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpvr11", 49, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "redr", 50, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rpid", 51, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rzpr", 52, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rtlbx", 53, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rtlbsx", 54, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rtlblo", 55, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rtlbhi", 56, 1, NULL, 32, "int");
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.microblaze.stack-protect");
+  tdesc_create_reg (feature, "rslr", 57, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "rshr", 58, 1, NULL, 32, "int");
+
+  tdesc_microblaze_linux_stack_protect = result;
+}
diff --git a/gdb/features/microblaze-linux-stack-protect.xml b/gdb/features/microblaze-linux-stack-protect.xml
new file mode 100644
index 0000000..29bbcfd
--- /dev/null
+++ b/gdb/features/microblaze-linux-stack-protect.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2014 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="microblaze-linux-core.xml"/>
+  <xi:include href="microblaze-stack-protect.xml"/>
+</target>
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 074d93d..6ba8efe 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -153,6 +153,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 \
@@ -364,6 +365,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
@@ -634,6 +636,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-linux-stack-protect.dat $(regdat_sh)
+	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/microblaze-linux-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..16e44ee 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -194,6 +194,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
+                        ;;
   nios2*-*-linux*)	srv_regobj="nios2-linux.o"
 			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
 			srv_xmlfiles="nios2-linux.xml"
diff --git a/gdb/gdbserver/linux-microblaze-low.c b/gdb/gdbserver/linux-microblaze-low.c
new file mode 100644
index 0000000..ed52d7f
--- /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_linux_stack_protect (void);
+extern const struct target_desc *tdesc_microblaze_linux_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	\
+  (sizeof microblaze_regmap / sizeof microblaze_regmap[0])
+
+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, "pc", &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, "pc", &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;
+}
+
+#ifdef HAVE_PTRACE_GETREGS
+
+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]);
+}
+
+#endif /* HAVE_PTRACE_GETREGS */
+
+static struct regset_info microblaze_regsets[] = {
+#ifdef HAVE_PTRACE_GETREGS
+  { 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 }
+};
+
+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_linux_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_linux_stack_protect ();
+
+  initialize_regsets_info (&microblaze_regsets_info);
+}
diff --git a/gdb/regformats/microblaze-linux-stack-protect.dat b/gdb/regformats/microblaze-linux-stack-protect.dat
new file mode 100644
index 0000000..342f94e
--- /dev/null
+++ b/gdb/regformats/microblaze-linux-stack-protect.dat
@@ -0,0 +1,63 @@
+# DO NOT EDIT: generated from microblaze-linux-stack-protect.xml
+name:microblaze_linux_stack_protect
+xmltarget:microblaze-linux-stack-protect.xml
+expedite:r1,pc
+32:r0
+32:r1
+32:r2
+32:r3
+32:r4
+32:r5
+32:r6
+32:r7
+32:r8
+32:r9
+32:r10
+32:r11
+32:r12
+32:r13
+32:r14
+32:r15
+32:r16
+32:r17
+32:r18
+32:r19
+32:r20
+32:r21
+32:r22
+32:r23
+32:r24
+32:r25
+32:r26
+32:r27
+32:r28
+32:r29
+32:r30
+32:r31
+32:pc
+32:rmsr
+32:rear
+32:resr
+32:rfsr
+32:rbtr
+32:rpvr0
+32:rpvr1
+32:rpvr2
+32:rpvr3
+32:rpvr4
+32:rpvr5
+32:rpvr6
+32:rpvr7
+32:rpvr8
+32:rpvr9
+32:rpvr10
+32:rpvr11
+32:redr
+32:rpid
+32:rzpr
+32:rtlbx
+32:rtlbsx
+32:rtlblo
+32:rtlbhi
+32:rslr
+32:rshr
-- 
1.7.1


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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:43       ` Joel Brobecker
  2014-09-10 15:04         ` Ajit Kumar Agarwal
@ 2014-09-12  8:01         ` Ajit Kumar Agarwal
  2014-09-12  8:39         ` Ajit Kumar Agarwal
  2 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-12  8:01 UTC (permalink / raw)
  To: Joel Brobecker, Michael Eager
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

With feedback comments incorporated, Please find the updated patch.

    [Patch, microblaze]: Port of Linux gdbserver

    This patch is the port of Linux gdbserver.

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

        * configure.tgt (build_gdbserver): New Definition.
        * features/microblaze-linux-core.xml: New file.
        * features/microblaze-linux-stack-protect.xml: New file.
        * features/microblaze-linux-stack-protect.c: New file.

    gdb/gdbserver/ChangeLog:

        * 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: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Wednesday, September 10, 2014 8:13 PM
To: Ajit Kumar Agarwal
Cc: Michael Eager; gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

[with my patch-champion hat]

I can't review the gdbserver Changes, but I can look at the rest.

>     ChangeLog:
>     2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
> 
>         * configure.host (microblaze): New.
>         (microblaze*-*-linux*): New.
>         * configure.tgt (build_gdbserver): New Definition.
>         * gdbserver/Makefile.in (microblaze-linux.c): New target.
>         * gdbserver/configure.srv (microblaze*-*-linux*): New target.
>         * gdbserver/linux-microblaze-low.c: New file.

Sorry Ajit. I should have seen this in the previous iteration, but there are a couple more nits in the ChangeLog entry that you'll need to fix.

First, gdb and gdb/gdbserver have distinct ChangeLog files, so you'll need to have 2 ChangeLog entries if you touch both areas at the same time.  This means that the filenames in gdbserver should not be prefixed with "gdbserver/" in your ChangeLog entry.

Also, change "ChangeLog:" to "gdb/ChangeLog:" and "gdbserver/ChangeLog:".

Please exclude the configure.host change. This looks unrelated.

> diff --git a/gdb/configure.tgt b/gdb/configure.tgt index 
> 01311b2..e4894da 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 
> 1447e61..b2a01f5 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -153,6 +153,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 \
> @@ -364,6 +365,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

There are indentation errors in the two hunks above. Use tabs.

> @@ -634,6 +636,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..16e44ee 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -194,6 +194,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
> +                        ;;
>    nios2*-*-linux*)	srv_regobj="nios2-linux.o"
>  			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
>  			srv_xmlfiles="nios2-linux.xml"
> diff --git a/gdb/gdbserver/linux-microblaze-low.c 
> b/gdb/gdbserver/linux-microblaze-low.c
> new file mode 100644
> index 0000000..b86b4f5
> --- /dev/null
> +++ b/gdb/gdbserver/linux-microblaze-low.c

It looks to me like you have many many trailing spaces in the file you are submitting. Please strip them.

Some lines are too long, also.

--
Joel

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 15:54           ` Joel Brobecker
@ 2014-09-10 16:10             ` Ajit Kumar Agarwal
  0 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 16:10 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Michael Eager, gdb-patches, Vinod Kathail, Vidhumouli Hunsigida,
	Nagaraju Mekala



-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Wednesday, September 10, 2014 9:25 PM
To: Ajit Kumar Agarwal
Cc: Michael Eager; gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

> >>Some lines are too long, also.
> 
> Joel: I have splitted the lines where it exceeds 80 Cols. I have 
> removed trailing spaces as much as I can. Could You please give  one 
> example of the trailing space which I have not removed.

>>Nearly every single line at the start of the file has trailing spaces.
>>I _really_ suggest you use an anchored search-and-replace to make sure you do not miss any; otherwise, you'll miss some if you do it by hand.

This suggestion is incredible and works like magic.

Thanks & Regards
Ajit
--
Joel

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 15:04         ` Ajit Kumar Agarwal
@ 2014-09-10 15:54           ` Joel Brobecker
  2014-09-10 16:10             ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Joel Brobecker @ 2014-09-10 15:54 UTC (permalink / raw)
  To: Ajit Kumar Agarwal
  Cc: Michael Eager, gdb-patches, Vinod Kathail, Vidhumouli Hunsigida,
	Nagaraju Mekala

> >>Some lines are too long, also.
> 
> Joel: I have splitted the lines where it exceeds 80 Cols. I have
> removed trailing spaces as much as I can. Could You please give  one
> example of the trailing space which I have not removed.

Nearly every single line at the start of the file has trailing spaces.
I _really_ suggest you use an anchored search-and-replace to make sure
you do not miss any; otherwise, you'll miss some if you do it by hand.

-- 
Joel

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 15:35               ` Michael Eager
@ 2014-09-10 15:51                 ` Ajit Kumar Agarwal
  0 siblings, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 15:51 UTC (permalink / raw)
  To: Michael Eager, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala




-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Wednesday, September 10, 2014 9:06 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/10/14 08:30, Ajit Kumar Agarwal wrote:
>
>>> Did you rebuild and test after making changes to the patch?
>
> I have compiled the patch and not tested as these are code formatting changes and not functionality changes.

>>All changes have to be tested.

Will do that.

>>Please do not top post.

Will do that.

>
> Thanks & Regards
> Ajit
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Wednesday, September 10, 2014 8:58 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/10/14 08:12, Ajit Kumar Agarwal wrote:
>> Sorry for the typo:
>>
>>> >From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.
>>
>>   From another terminal gdb compiled with  microblazeel-xilinx-linux-gnu is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.
>
> Answer ALL questions:
>
> Did you rebuild and test after making changes to the patch?
>


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 15:30             ` Ajit Kumar Agarwal
@ 2014-09-10 15:35               ` Michael Eager
  2014-09-10 15:51                 ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-09-10 15:35 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/10/14 08:30, Ajit Kumar Agarwal wrote:
>
>>> Did you rebuild and test after making changes to the patch?
>
> I have compiled the patch and not tested as these are code formatting changes and not functionality changes.

All changes have to be tested.

Please do not top post.

>
> Thanks & Regards
> Ajit
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Wednesday, September 10, 2014 8:58 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/10/14 08:12, Ajit Kumar Agarwal wrote:
>> Sorry for the typo:
>>
>>> >From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.
>>
>>   From another terminal gdb compiled with  microblazeel-xilinx-linux-gnu is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.
>
> Answer ALL questions:
>
> Did you rebuild and test after making changes to the patch?
>


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 15:27           ` Michael Eager
@ 2014-09-10 15:30             ` Ajit Kumar Agarwal
  2014-09-10 15:35               ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 15:30 UTC (permalink / raw)
  To: Michael Eager, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala


>>Did you rebuild and test after making changes to the patch?

I have compiled the patch and not tested as these are code formatting changes and not functionality changes.

Thanks & Regards
Ajit
-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com] 
Sent: Wednesday, September 10, 2014 8:58 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/10/14 08:12, Ajit Kumar Agarwal wrote:
> Sorry for the typo:
>
>> >From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.
>
>  From another terminal gdb compiled with  microblazeel-xilinx-linux-gnu is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.

Answer ALL questions:

Did you rebuild and test after making changes to the patch?

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 15:12         ` Ajit Kumar Agarwal
@ 2014-09-10 15:27           ` Michael Eager
  2014-09-10 15:30             ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-09-10 15:27 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/10/14 08:12, Ajit Kumar Agarwal wrote:
> Sorry for the typo:
>
>> >From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.
>
>  From another terminal gdb compiled with  microblazeel-xilinx-linux-gnu is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.

Answer ALL questions:

Did you rebuild and test after making changes to the patch?

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:39       ` Michael Eager
  2014-09-10 14:59         ` Ajit Kumar Agarwal
@ 2014-09-10 15:12         ` Ajit Kumar Agarwal
  2014-09-10 15:27           ` Michael Eager
  1 sibling, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 15:12 UTC (permalink / raw)
  To: Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

Sorry for the typo:

>>From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.

From another terminal gdb compiled with  microblazeel-xilinx-linux-gnu is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.

Thanks & Regards
Ajit
-----Original Message-----
From: Ajit Kumar Agarwal 
Sent: Wednesday, September 10, 2014 8:30 PM
To: 'Michael Eager'; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: RE: [Patch, microblaze]: Port of Linux gdbserver

>> How has this been tested?   Please be specific.

The changes are tested with the Xilinx PetaLinux. The Petalinux is booted up with QEMU and the gdbserver is run as follows.

/bin/gdbserver HOST:1234 application.elf.

From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and Break point is set and continue.

All the testing has been done with little-endian binaries. Even the Linux kernel that got booted up are little endian. 

Rest my answers are inlined below.

Thanks & Regards
Ajit

-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com]
Sent: Wednesday, September 10, 2014 8:09 PM
To: Ajit Kumar Agarwal; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

On 09/10/14 07:22, Ajit Kumar Agarwal wrote:
> Please find the updated patch with review feedbacks are incorporated.
>
>     [Patch, microblaze]: Port of Linux gdbserver
>
>      This patch is the port of Linux gdbserver.
>
>      ChangeLog:
>      2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
>
>          * configure.host (microblaze): New.
>          (microblaze*-*-linux*): New.
>          * configure.tgt (build_gdbserver): New Definition.
>          * 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

Please review your patches before you (re)submit them.

+  /* If necessary, recognize more trap instructions here.  GDB only uses the
+      one.  */

>>One line.

 If this exceeds the 80 Cols then also it should be in one Line.
+  else
+    {
+        if (size < sizeof (long))
+          supply_register (regcache, regno, buf + sizeof (long) - size);
+        else
+          supply_register (regcache, regno, buf);
+    }
+}

>>Fix indent.

Could you please explain what indentation is required in the above case.

+static struct usrregs_info microblaze_usrregs_info = {
+   microblaze_num_regs,
+   microblaze_regmap,
+};

>>Fix indent.

I will fix this.

>>How has this been tested?   Please be specific.

Answered above.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:43       ` Joel Brobecker
@ 2014-09-10 15:04         ` Ajit Kumar Agarwal
  2014-09-10 15:54           ` Joel Brobecker
  2014-09-12  8:01         ` Ajit Kumar Agarwal
  2014-09-12  8:39         ` Ajit Kumar Agarwal
  2 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 15:04 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Michael Eager, gdb-patches, Vinod Kathail, Vidhumouli Hunsigida,
	Nagaraju Mekala



-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Wednesday, September 10, 2014 8:13 PM
To: Ajit Kumar Agarwal
Cc: Michael Eager; gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

[with my patch-champion hat]

I can't review the gdbserver Changes, but I can look at the rest.

>     ChangeLog:
>     2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
> 
>         * configure.host (microblaze): New.
>         (microblaze*-*-linux*): New.
>         * configure.tgt (build_gdbserver): New Definition.
>         * gdbserver/Makefile.in (microblaze-linux.c): New target.
>         * gdbserver/configure.srv (microblaze*-*-linux*): New target.
>         * gdbserver/linux-microblaze-low.c: New file.

Sorry Ajit. I should have seen this in the previous iteration, but there are a couple more nits in the ChangeLog entry that you'll need to fix.

>>First, gdb and gdb/gdbserver have distinct ChangeLog files, so you'll need to have 2 ChangeLog entries if you touch both areas at the same time.  This >>means that the filenames in gdbserver should not be prefixed with "gdbserver/" in your ChangeLog entry.

>>Also, change "ChangeLog:" to "gdb/ChangeLog:" and "gdbserver/ChangeLog:".

>>Please exclude the configure.host change. This looks unrelated.

I will incorporate this.
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt index 
> 01311b2..e4894da 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 
> 1447e61..b2a01f5 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -153,6 +153,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 \
> @@ -364,6 +365,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

>>There are indentation errors in the two hunks above. Use tabs.

I will do this.

> @@ -634,6 +636,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..16e44ee 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -194,6 +194,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
> +                        ;;
>    nios2*-*-linux*)	srv_regobj="nios2-linux.o"
>  			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
>  			srv_xmlfiles="nios2-linux.xml"
> diff --git a/gdb/gdbserver/linux-microblaze-low.c 
> b/gdb/gdbserver/linux-microblaze-low.c
> new file mode 100644
> index 0000000..b86b4f5
> --- /dev/null
> +++ b/gdb/gdbserver/linux-microblaze-low.c

>>It looks to me like you have many many trailing spaces in the file you are submitting. Please strip them.

>>Some lines are too long, also.

Joel: I have splitted the lines where it exceeds 80 Cols. I have removed trailing spaces as much as I can. Could 
You please give  one example of the trailing space which I have not removed.

Thanks & Regards
Ajit
--
Joel

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:39       ` Michael Eager
@ 2014-09-10 14:59         ` Ajit Kumar Agarwal
  2014-09-10 15:12         ` Ajit Kumar Agarwal
  1 sibling, 0 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 14:59 UTC (permalink / raw)
  To: Michael Eager, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

>> How has this been tested?   Please be specific.

The changes are tested with the Xilinx PetaLinux. The Petalinux is booted up with QEMU and the gdbserver is run as follows.

/bin/gdbserver HOST:1234 application.elf.

From another terminal mb-gdb is run and attached to gdbserver process run before using tar remote command and 
Break point is set and continue.

All the testing has been done with little-endian binaries. Even the Linux kernel that got booted up are little endian. 

Rest my answers are inlined below.

Thanks & Regards
Ajit

-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Wednesday, September 10, 2014 8:09 PM
To: Ajit Kumar Agarwal; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

On 09/10/14 07:22, Ajit Kumar Agarwal wrote:
> Please find the updated patch with review feedbacks are incorporated.
>
>     [Patch, microblaze]: Port of Linux gdbserver
>
>      This patch is the port of Linux gdbserver.
>
>      ChangeLog:
>      2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
>
>          * configure.host (microblaze): New.
>          (microblaze*-*-linux*): New.
>          * configure.tgt (build_gdbserver): New Definition.
>          * 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

Please review your patches before you (re)submit them.

+  /* If necessary, recognize more trap instructions here.  GDB only uses the
+      one.  */

>>One line.

 If this exceeds the 80 Cols then also it should be in one Line.
+  else
+    {
+        if (size < sizeof (long))
+          supply_register (regcache, regno, buf + sizeof (long) - size);
+        else
+          supply_register (regcache, regno, buf);
+    }
+}

>>Fix indent.

Could you please explain what indentation is required in the above case.

+static struct usrregs_info microblaze_usrregs_info = {
+   microblaze_num_regs,
+   microblaze_regmap,
+};

>>Fix indent.

I will fix this.

>>How has this been tested?   Please be specific.

Answered above.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:23     ` Ajit Kumar Agarwal
  2014-09-10 14:39       ` Michael Eager
@ 2014-09-10 14:43       ` Joel Brobecker
  2014-09-10 15:04         ` Ajit Kumar Agarwal
                           ` (2 more replies)
  1 sibling, 3 replies; 59+ messages in thread
From: Joel Brobecker @ 2014-09-10 14:43 UTC (permalink / raw)
  To: Ajit Kumar Agarwal
  Cc: Michael Eager, gdb-patches, Vinod Kathail, Vidhumouli Hunsigida,
	Nagaraju Mekala

[with my patch-champion hat]

I can't review the gdbserver Changes, but I can look at the rest.

>     ChangeLog:
>     2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
> 
>         * configure.host (microblaze): New.
>         (microblaze*-*-linux*): New.
>         * configure.tgt (build_gdbserver): New Definition.
>         * gdbserver/Makefile.in (microblaze-linux.c): New target.
>         * gdbserver/configure.srv (microblaze*-*-linux*): New target.
>         * gdbserver/linux-microblaze-low.c: New file.

Sorry Ajit. I should have seen this in the previous iteration, but
there are a couple more nits in the ChangeLog entry that you'll need
to fix.

First, gdb and gdb/gdbserver have distinct ChangeLog files, so you'll
need to have 2 ChangeLog entries if you touch both areas at the same
time.  This means that the filenames in gdbserver should not be
prefixed with "gdbserver/" in your ChangeLog entry.

Also, change "ChangeLog:" to "gdb/ChangeLog:" and
"gdbserver/ChangeLog:".

Please exclude the configure.host change. This looks unrelated.

> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 01311b2..e4894da 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 1447e61..b2a01f5 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -153,6 +153,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 \
> @@ -364,6 +365,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

There are indentation errors in the two hunks above. Use tabs.

> @@ -634,6 +636,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..16e44ee 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -194,6 +194,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
> +                        ;;
>    nios2*-*-linux*)	srv_regobj="nios2-linux.o"
>  			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
>  			srv_xmlfiles="nios2-linux.xml"
> diff --git a/gdb/gdbserver/linux-microblaze-low.c b/gdb/gdbserver/linux-microblaze-low.c
> new file mode 100644
> index 0000000..b86b4f5
> --- /dev/null
> +++ b/gdb/gdbserver/linux-microblaze-low.c

It looks to me like you have many many trailing spaces in the file
you are submitting. Please strip them.

Some lines are too long, also.

-- 
Joel

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 14:23     ` Ajit Kumar Agarwal
@ 2014-09-10 14:39       ` Michael Eager
  2014-09-10 14:59         ` Ajit Kumar Agarwal
  2014-09-10 15:12         ` Ajit Kumar Agarwal
  2014-09-10 14:43       ` Joel Brobecker
  1 sibling, 2 replies; 59+ messages in thread
From: Michael Eager @ 2014-09-10 14:39 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, Joel Brobecker
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/10/14 07:22, Ajit Kumar Agarwal wrote:
> Please find the updated patch with review feedbacks are incorporated.
>
>     [Patch, microblaze]: Port of Linux gdbserver
>
>      This patch is the port of Linux gdbserver.
>
>      ChangeLog:
>      2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
>
>          * configure.host (microblaze): New.
>          (microblaze*-*-linux*): New.
>          * configure.tgt (build_gdbserver): New Definition.
>          * 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

Please review your patches before you (re)submit them.

+  /* If necessary, recognize more trap instructions here.  GDB only uses the
+      one.  */

One line.

+  else
+    {
+        if (size < sizeof (long))
+          supply_register (regcache, regno, buf + sizeof (long) - size);
+        else
+          supply_register (regcache, regno, buf);
+    }
+}

Fix indent.

+static struct usrregs_info microblaze_usrregs_info =
+{
+   microblaze_num_regs,
+   microblaze_regmap,
+};

Fix indent.

How has this been tested?   Please be specific.

Did you rebuild and test after making changes to the patch?


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* RE: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 13:46   ` Joel Brobecker
@ 2014-09-10 14:23     ` Ajit Kumar Agarwal
  2014-09-10 14:39       ` Michael Eager
  2014-09-10 14:43       ` Joel Brobecker
  0 siblings, 2 replies; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 14:23 UTC (permalink / raw)
  To: Joel Brobecker, Michael Eager
  Cc: gdb-patches, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

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

Please find the updated patch with review feedbacks are incorporated.

   [Patch, microblaze]: Port of Linux gdbserver

    This patch is the port of Linux gdbserver.

    ChangeLog:
    2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>

        * configure.host (microblaze): New.
        (microblaze*-*-linux*): New.
        * configure.tgt (build_gdbserver): New Definition.
        * 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: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Wednesday, September 10, 2014 7:16 PM
To: Michael Eager
Cc: Ajit Kumar Agarwal; gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Port of Linux gdbserver

> >     This patch is the port of Linux gdbserver.
> >
> >     ChangeLog:
> >     2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
> >
> >         * microblaze-tdep.h (MICROBLAZE_BREAKPOINT): New Macro.
> >         (MICROBLAZE_BREAKPOINT_LE): New Macro.
> >         * microblaze-linux-tdep.c
> >         (microblaze_linux_memory_remove_breakpoint): Use of
> >         set_gdbarch_fetch_tls_load_module_address, do_cleanup.
> >         * configure.host (microblaze): New.
> >         (microblaze*-*-linux*): New.
> >         * configure.tgt (build_gdbserver): New Definition.
> >         * gdbserver/Makefile.in (microblaze-linux.c): New target.
> >         * gdbserver/configure.srv (microblaze*-*-linux*): New target.
> >         * gdbserver/linux-microblaze-low.c: New file.

In addition to Michael's comment, I am not sure why the gdb changes are combined with the gdbserver changes. Shouldn't these be submitted separately?

--
Joel

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

From 0cb7626ea548b85b41e83da9a23c5bcba632c597 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Wed, 10 Sep 2014 19:43:53 +0530
Subject: [PATCH] [Patch, microblaze]: Port of Linux gdbserver

This patch is the port of Linux gdbserver.

ChangeLog:
2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>

        * configure.host (microblaze): New.
        (microblaze*-*-linux*): New.
        * configure.tgt (build_gdbserver): New Definition.
        * 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.host                   |    3 +
 gdb/configure.tgt                    |    1 +
 gdb/gdbserver/Makefile.in            |    4 +
 gdb/gdbserver/configure.srv          |    6 +
 gdb/gdbserver/linux-microblaze-low.c |  236 ++++++++++++++++++++++++++++++++++
 5 files changed, 250 insertions(+), 0 deletions(-)
 create mode 100644 gdb/gdbserver/linux-microblaze-low.c

diff --git a/gdb/configure.host b/gdb/configure.host
index 15a8288..6f02ddd 100644
--- a/gdb/configure.host
+++ b/gdb/configure.host
@@ -59,6 +59,7 @@ i[34567]86*)		gdb_host_cpu=i386 ;;
 m68*)			gdb_host_cpu=m68k ;;
 m88*)			gdb_host_cpu=m88k ;;
 mips*)			gdb_host_cpu=mips ;;
+microblaze*)            gdb_host_cpu=microblaze ;;
 powerpc* | rs6000)	gdb_host_cpu=powerpc ;;
 sparcv9 | sparc64)	gdb_host_cpu=sparc ;;
 s390*)			gdb_host_cpu=s390 ;;
@@ -133,6 +134,8 @@ mips*-*-netbsd* | mips*-*-knetbsd*-gnu)
 			gdb_host=nbsd ;;
 mips64*-*-openbsd*)	gdb_host=obsd64 ;;
 
+microblaze*-*linux*)    gdb_host=linux ;;
+
 powerpc-*-aix* | rs6000-*-* | powerpc64-*-aix*)
 			gdb_host=aix ;;
 powerpc*-*-freebsd*)	gdb_host=fbsd ;;
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 01311b2..e4894da 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 1447e61..b2a01f5 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -153,6 +153,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 \
@@ -364,6 +365,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
@@ -634,6 +636,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..16e44ee 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -194,6 +194,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
+                        ;;
   nios2*-*-linux*)	srv_regobj="nios2-linux.o"
 			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
 			srv_xmlfiles="nios2-linux.xml"
diff --git a/gdb/gdbserver/linux-microblaze-low.c b/gdb/gdbserver/linux-microblaze-low.c
new file mode 100644
index 0000000..b86b4f5
--- /dev/null
+++ b/gdb/gdbserver/linux-microblaze-low.c
@@ -0,0 +1,236 @@
+/* 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 <asm/ptrace.h>
+#include <sys/procfs.h>
+ 
+#include "gdb_proc_service.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 (sizeof microblaze_regmap / sizeof microblaze_regmap[0])
+ 
+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, "pc", &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, "pc", &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;
+
+  /* If necessary, recognize more trap instructions here.  GDB only uses the
+      one.  */
+  return 0;
+}
+ 
+static CORE_ADDR
+microblaze_reinsert_addr (struct regcache *regcache)
+{
+  unsigned long pc;
+  collect_register_by_name (regcache, "r15", &pc);
+  return pc;
+}
+ 
+#ifdef HAVE_PTRACE_GETREGS
+ 
+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]);
+}
+ 
+#endif /* HAVE_PTRACE_GETREGS */
+
+static struct regset_info microblaze_regsets[] = {
+#ifdef HAVE_PTRACE_GETREGS
+  { 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 }
+}; 
+
+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


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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 13:31 ` Michael Eager
@ 2014-09-10 13:46   ` Joel Brobecker
  2014-09-10 14:23     ` Ajit Kumar Agarwal
  0 siblings, 1 reply; 59+ messages in thread
From: Joel Brobecker @ 2014-09-10 13:46 UTC (permalink / raw)
  To: Michael Eager
  Cc: Ajit Kumar Agarwal, gdb-patches, Vinod Kathail,
	Vidhumouli Hunsigida, Nagaraju Mekala

> >     This patch is the port of Linux gdbserver.
> >
> >     ChangeLog:
> >     2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
> >
> >         * microblaze-tdep.h (MICROBLAZE_BREAKPOINT): New Macro.
> >         (MICROBLAZE_BREAKPOINT_LE): New Macro.
> >         * microblaze-linux-tdep.c
> >         (microblaze_linux_memory_remove_breakpoint): Use of
> >         set_gdbarch_fetch_tls_load_module_address, do_cleanup.
> >         * configure.host (microblaze): New.
> >         (microblaze*-*-linux*): New.
> >         * configure.tgt (build_gdbserver): New Definition.
> >         * gdbserver/Makefile.in (microblaze-linux.c): New target.
> >         * gdbserver/configure.srv (microblaze*-*-linux*): New target.
> >         * gdbserver/linux-microblaze-low.c: New file.

In addition to Michael's comment, I am not sure why the gdb changes
are combined with the gdbserver changes. Shouldn't these be submitted
separately?

-- 
Joel

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

* Re: [Patch, microblaze]: Port of Linux gdbserver
  2014-09-10 10:14 [Patch, microblaze]: " Ajit Kumar Agarwal
@ 2014-09-10 13:31 ` Michael Eager
  2014-09-10 13:46   ` Joel Brobecker
  0 siblings, 1 reply; 59+ messages in thread
From: Michael Eager @ 2014-09-10 13:31 UTC (permalink / raw)
  To: Ajit Kumar Agarwal, gdb-patches
  Cc: Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

On 09/10/14 03:14, Ajit Kumar Agarwal wrote:
> Please find the patch for the Linux gdbserver port for Microblaze.
>
> [Patch, microblaze]: Port of Linux gdbserver
>
>      This patch is the port of Linux gdbserver.
>
>      ChangeLog:
>      2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>
>
>          * microblaze-tdep.h (MICROBLAZE_BREAKPOINT): New Macro.
>          (MICROBLAZE_BREAKPOINT_LE): New Macro.
>          * microblaze-linux-tdep.c
>          (microblaze_linux_memory_remove_breakpoint): Use of
>          set_gdbarch_fetch_tls_load_module_address, do_cleanup.
>          * configure.host (microblaze): New.
>          (microblaze*-*-linux*): New.
>          * configure.tgt (build_gdbserver): New Definition.
>          * 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

Please follow GNU coding conventions:

+  if (microblaze_regmap[regno] == -1 || regno == 0)
+    return 1;
+
+    return 0;

No indent.

+/* dbtrap insn */
+/* brki r16, 0x18; */

Remove cruft.

+  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 {

Indent.

+  for (i = 0; i < microblaze_num_regs; i++)
+    microblaze_collect_ptrace_register (regcache, i, (char *) buf + microblaze_regmap[i]);

Break lines before 80 cols.

+static struct regsets_info microblaze_regsets_info = {
+
+  microblaze_regsets, /* regsets */
+  0, /* num_regsets */
+  NULL, /* disabled_regsets */
+};

Remove blank line.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

* [Patch, microblaze]: Port of Linux gdbserver
@ 2014-09-10 10:14 Ajit Kumar Agarwal
  2014-09-10 13:31 ` Michael Eager
  0 siblings, 1 reply; 59+ messages in thread
From: Ajit Kumar Agarwal @ 2014-09-10 10:14 UTC (permalink / raw)
  To: gdb-patches
  Cc: Michael Eager, Vinod Kathail, Vidhumouli Hunsigida, Nagaraju Mekala

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

Please find the patch for the Linux gdbserver port for Microblaze.    

[Patch, microblaze]: Port of Linux gdbserver

    This patch is the port of Linux gdbserver.

    ChangeLog:
    2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>

        * microblaze-tdep.h (MICROBLAZE_BREAKPOINT): New Macro.
        (MICROBLAZE_BREAKPOINT_LE): New Macro.
        * microblaze-linux-tdep.c
        (microblaze_linux_memory_remove_breakpoint): Use of
        set_gdbarch_fetch_tls_load_module_address, do_cleanup.
        * configure.host (microblaze): New.
        (microblaze*-*-linux*): New.
        * configure.tgt (build_gdbserver): New Definition.
        * 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

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

From 207ef96a198bfdabf3efa6ad3d12e6fddbda2964 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Wed, 10 Sep 2014 15:19:54 +0530
Subject: [PATCH] [Patch, microblaze]: Port of Linux gdbserver

This patch is the port of Linux gdbserver.

ChangeLog:
2014-10-09  Ajit Agarwal  <ajitkum@xilinx.com>

	* microblaze-tdep.h (MICROBLAZE_BREAKPOINT): New Macro.
	(MICROBLAZE_BREAKPOINT_LE): New Macro.
	* microblaze-linux-tdep.c
	(microblaze_linux_memory_remove_breakpoint): Use of
	set_gdbarch_fetch_tls_load_module_address, do_cleanup.
	* configure.host (microblaze): New.
	(microblaze*-*-linux*): New.
	* configure.tgt (build_gdbserver): New Definition.
	* 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.host                   |    3 +
 gdb/configure.tgt                    |    1 +
 gdb/gdbserver/Makefile.in            |    4 +
 gdb/gdbserver/configure.srv          |    6 +
 gdb/gdbserver/linux-microblaze-low.c |  236 ++++++++++++++++++++++++++++++++++
 gdb/microblaze-linux-tdep.c          |   11 ++
 gdb/microblaze-tdep.h                |    4 +-
 7 files changed, 264 insertions(+), 1 deletions(-)
 create mode 100644 gdb/gdbserver/linux-microblaze-low.c

diff --git a/gdb/configure.host b/gdb/configure.host
index 15a8288..6f02ddd 100644
--- a/gdb/configure.host
+++ b/gdb/configure.host
@@ -59,6 +59,7 @@ i[34567]86*)		gdb_host_cpu=i386 ;;
 m68*)			gdb_host_cpu=m68k ;;
 m88*)			gdb_host_cpu=m88k ;;
 mips*)			gdb_host_cpu=mips ;;
+microblaze*)            gdb_host_cpu=microblaze ;;
 powerpc* | rs6000)	gdb_host_cpu=powerpc ;;
 sparcv9 | sparc64)	gdb_host_cpu=sparc ;;
 s390*)			gdb_host_cpu=s390 ;;
@@ -133,6 +134,8 @@ mips*-*-netbsd* | mips*-*-knetbsd*-gnu)
 			gdb_host=nbsd ;;
 mips64*-*-openbsd*)	gdb_host=obsd64 ;;
 
+microblaze*-*linux*)    gdb_host=linux ;;
+
 powerpc-*-aix* | rs6000-*-* | powerpc64-*-aix*)
 			gdb_host=aix ;;
 powerpc*-*-freebsd*)	gdb_host=fbsd ;;
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 01311b2..e4894da 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 1447e61..b2a01f5 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -153,6 +153,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 \
@@ -364,6 +365,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
@@ -634,6 +636,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..16e44ee 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -194,6 +194,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
+                        ;;
   nios2*-*-linux*)	srv_regobj="nios2-linux.o"
 			srv_tgtobj="$srv_linux_obj linux-nios2-low.o"
 			srv_xmlfiles="nios2-linux.xml"
diff --git a/gdb/gdbserver/linux-microblaze-low.c b/gdb/gdbserver/linux-microblaze-low.c
new file mode 100644
index 0000000..ac6b998
--- /dev/null
+++ b/gdb/gdbserver/linux-microblaze-low.c
@@ -0,0 +1,236 @@
+/* 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 <asm/ptrace.h>
+#include <sys/procfs.h>
+ 
+#include "gdb_proc_service.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 (sizeof microblaze_regmap / sizeof microblaze_regmap[0])
+ 
+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, "pc", &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, "pc", &newpc);
+}
+ 
+/* dbtrap insn */
+/* brki r16, 0x18; */
+ 
+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;
+
+  /* If necessary, recognize more trap instructions here.  GDB only uses the
+      one.  */
+   return 0;
+}
+ 
+static CORE_ADDR
+microblaze_reinsert_addr (struct regcache *regcache)
+{
+  unsigned long pc;
+  collect_register_by_name (regcache, "r15", &pc);
+  return pc;
+}
+ 
+#ifdef HAVE_PTRACE_GETREGS
+ 
+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]);
+}
+ 
+#endif /* HAVE_PTRACE_GETREGS */
+
+static struct regset_info microblaze_regsets[] = {
+#ifdef HAVE_PTRACE_GETREGS
+  { 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 }
+}; 
+
+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);
+}
diff --git a/gdb/microblaze-linux-tdep.c b/gdb/microblaze-linux-tdep.c
index 8d360eb..af4d31c 100644
--- a/gdb/microblaze-linux-tdep.c
+++ b/gdb/microblaze-linux-tdep.c
@@ -46,12 +46,16 @@ microblaze_linux_memory_remove_breakpoint (struct gdbarch *gdbarch,
   int val;
   int bplen;
   gdb_byte old_contents[BREAKPOINT_MAX];
+  struct cleanup *cleanup;
 
   /* Determine appropriate breakpoint contents and size for this address.  */
   bp = gdbarch_breakpoint_from_pc (gdbarch, &addr, &bplen);
   if (bp == NULL)
     error (_("Software breakpoints not implemented for this target."));
 
+  /* Make sure we see the memory breakpoints.  */ 
+  cleanup = make_show_memory_breakpoints_cleanup (1); 
+
   val = target_read_memory (addr, old_contents, bplen);
 
   /* If our breakpoint is no longer at the address, this means that the
@@ -60,6 +64,7 @@ microblaze_linux_memory_remove_breakpoint (struct gdbarch *gdbarch,
   if (val == 0 && memcmp (bp, old_contents, bplen) == 0)
     val = target_write_raw_memory (addr, bp_tgt->shadow_contents, bplen);
 
+  do_cleanups (cleanup);
   return val;
 }
 
@@ -135,6 +140,12 @@ microblaze_linux_init_abi (struct gdbarch_info info,
   /* Trampolines.  */
   tramp_frame_prepend_unwinder (gdbarch,
 				&microblaze_linux_sighandler_tramp_frame);
+  
+   /* Enable TLS support.  */ 
+   set_gdbarch_fetch_tls_load_module_address (gdbarch, 
+                                              svr4_fetch_objfile_link_map); 
+    
+
 }
 
 /* -Wmissing-prototypes */
diff --git a/gdb/microblaze-tdep.h b/gdb/microblaze-tdep.h
index ba240e9..574197c 100644
--- a/gdb/microblaze-tdep.h
+++ b/gdb/microblaze-tdep.h
@@ -117,6 +117,8 @@ struct microblaze_frame_cache
 
 /* MICROBLAZE_BREAKPOINT defines the breakpoint that should be used.
    Only used for native debugging.  */
-#define MICROBLAZE_BREAKPOINT {0xb9, 0xcc, 0x00, 0x60}
+#define MICROBLAZE_BREAKPOINT {0xba, 0x0c, 0x00, 0x18} 
+#define MICROBLAZE_BREAKPOINT_LE {0x18, 0x00, 0x0c, 0xba 
+
 
 #endif /* microblaze-tdep.h */
-- 
1.7.1


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

end of thread, other threads:[~2014-12-19 18:06 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-08 13:52 [Patch] Microblaze: Port of Linux gdbserver Ajit Kumar Agarwal
2014-10-09 16:29 ` Michael Eager
2014-10-09 18:54   ` Ajit Kumar Agarwal
2014-10-09 23:42     ` Michael Eager
2014-10-13 16:00       ` Ajit Kumar Agarwal
2014-10-13 17:49         ` Michael Eager
2014-10-14  3:03           ` Ajit Kumar Agarwal
2014-10-14 15:07             ` Michael Eager
2014-10-14 15:33               ` Ajit Kumar Agarwal
2014-10-14 15:42               ` Ajit Kumar Agarwal
2014-10-15 13:27     ` Pedro Alves
2014-10-17 19:22       ` Ajit Kumar Agarwal
2014-12-15 18:02         ` Pedro Alves
2014-12-15 18:13           ` Michael Eager
2014-12-18  8:58             ` Ajit Kumar Agarwal
2014-12-18 16:10               ` Michael Eager
2014-12-18  8:57           ` Ajit Kumar Agarwal
2014-12-18 11:28             ` Pedro Alves
2014-12-18 16:53               ` Ajit Kumar Agarwal
2014-12-18 17:40                 ` Pedro Alves
2014-12-19  8:27                   ` Ajit Kumar Agarwal
2014-12-19 10:56                     ` Pedro Alves
2014-12-19 10:26               ` Ajit Kumar Agarwal
2014-12-19 11:02                 ` Pedro Alves
2014-12-19 18:06                   ` Ajit Kumar Agarwal
2014-11-26 12:13       ` Ajit Kumar Agarwal
2014-12-15 16:08         ` Ajit Kumar Agarwal
  -- strict thread matches above, loose matches on Subject: below --
2014-10-08 14:59 Ajit Kumar Agarwal
2014-09-10 10:14 [Patch, microblaze]: " Ajit Kumar Agarwal
2014-09-10 13:31 ` Michael Eager
2014-09-10 13:46   ` Joel Brobecker
2014-09-10 14:23     ` Ajit Kumar Agarwal
2014-09-10 14:39       ` Michael Eager
2014-09-10 14:59         ` Ajit Kumar Agarwal
2014-09-10 15:12         ` Ajit Kumar Agarwal
2014-09-10 15:27           ` Michael Eager
2014-09-10 15:30             ` Ajit Kumar Agarwal
2014-09-10 15:35               ` Michael Eager
2014-09-10 15:51                 ` Ajit Kumar Agarwal
2014-09-10 14:43       ` Joel Brobecker
2014-09-10 15:04         ` Ajit Kumar Agarwal
2014-09-10 15:54           ` Joel Brobecker
2014-09-10 16:10             ` Ajit Kumar Agarwal
2014-09-12  8:01         ` Ajit Kumar Agarwal
2014-09-12  8:39         ` Ajit Kumar Agarwal
2014-09-12 15:38           ` Michael Eager
2014-09-16  6:42             ` Ajit Kumar Agarwal
2014-09-16 12:06               ` Michael Eager
2014-09-17  9:36                 ` Ajit Kumar Agarwal
2014-09-17 14:12                   ` Michael Eager
2014-09-16 17:04               ` Pedro Alves
2014-09-17  6:16                 ` Ajit Kumar Agarwal
2014-09-17  8:15                   ` Pedro Alves
2014-09-17  8:20                     ` Ajit Kumar Agarwal
2014-09-23 12:49                     ` Ajit Kumar Agarwal
2014-09-30 11:43                       ` Pedro Alves
2014-09-30 13:27                         ` Ajit Kumar Agarwal
2014-09-30 13:37                           ` Pedro Alves
2014-09-30 14:21                             ` Ajit Kumar Agarwal

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