RE: [Patch, microblaze]: Add slr and shr regs and little-endian breakpoint

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

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

Thanks  Joel for the review comments. Will make sure to consider your suggestions
 when Submitting the new patches.
Based on the feedback, the updated patch is given below.

Okay for the upstream?

    [Patch, microblaze]: Add slr and shr regs
    
    This patch add the support of slr and shr regs and also solves the problem
    related to process_g_packet where the buf_len > 2 * rsa->sizeof_g_packet
    and throwing the Error that 'g' packet message reply is too long. This is
    because the buf_len calculated in the init_remote_state function for
    microblaze target is based On the sizeof_g_packet and remote_packet_size
    and the memory_packet_config->size. The sizeof_g_packet is 236 because the
    number of reg num is 59 and 2* sizeof_g_packet comes to 472 .With shr and
    shl entry and the buf_len is 472. This does not match the greater than
    conditional statement  and works fine. Without shr and shl entry,the
    sizeof_g_packets comes to 57*4 *2 = 456.  This doesn't match the criteria
    in the process_g_packet function  leading to throwing of error message as
    " 'g' packet message reply is too long".
    
    ChangeLog:
    2014-05-20 Ajit Agarwal <ajitkum@xilinx.com>
    
        * gdb/gdbserver/Makefile.in (microblaze-linux.c): New rule.
    
        * gdb/microblaze-tdep.c (microblaze_register_names): Added
        the rshr and rslr register names.
    
        * gdb/microblaze-tdep.h (microblaze_reg_num): Addition of
        field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM.
        (microblaze_frame_cache): Change in the index of
        register_offsets.
    
        * gdb/regformats/reg-microblaze.dat: New Register data file.
    
    Signed-off-by:Ajit Agarwal ajitkum@xilinx.com

Thanks & Regards
Ajit

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com] 
Sent: Wednesday, May 21, 2014 7:16 PM
To: Ajit Kumar Agarwal
Cc: Michael Eager; Michael Eager; gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs and little-endian breakpoint

> Here is the problem  description. The problem is related to 
> process_g_packet where the buf_len > 2 * rsa->sizeof_g_packet and 
> throwing the Error that 'g' packet message reply is too long. This is 
> because the buf_len calculated in the init_remote_state function for 
> microblaze target is based On the sizeof_g_packet and 
> remote_packet_size and the memory_packet_config->size. The 
> sizeof_g_packet is 236 because the number of reg num is 59 and 2* 
> sizeof_g_packet comes to 472 .With shr and shl entry and the buf_len 
> is 472. This does not match the greater than conditional statement and 
> works fine. Without shr and shl entry,the sizeof_g_packets comes to
> 57*4 *2 = 456.  This doesn't match the criteria in the 
> process_g_packet function leading to throwing of error message as  "
> 'g' packet message reply is too long".
> 
> This is happening when tar remote machine:1234 command is run in gdb.
> 
> >>>These changes don't appear related.  Split independent changes in 
> >>>to separate patches.
> 
> I will split the changes  and send the separate patch with shr and shl 
> reg changes and the little endian breakpoint support.

Thanks, Ajit. FWIW, I agree with Michael.

Also, when submitting the new patches, please include the description of the problem you are trying to solve in the revision log of your patch. Something like the description you gave above, for instance.
This makes archeology a lot easier for us.  The good news when doing that is that submitting the patch could be as simple as "git send-email"-ing the patch; that's what I personally do for my own patches, and it's been a real time saver.

--
Joel

[-- Attachment #2: 0001-Patch-microblaze-Add-slr-and-shr-regs.patch --]
[-- Type: application/octet-stream, Size: 5022 bytes --]

From a939e32e0012fb611fcc7831750ddd925e9718b1 Mon Sep 17 00:00:00 2001
From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)>
Date: Wed, 21 May 2014 18:55:23 +0530
Subject: [PATCH] [Patch, microblaze]: Add slr and shr regs

This patch add the support of slr and shr regs and also solves the problem
related to process_g_packet where the buf_len > 2 * rsa->sizeof_g_packet
and throwing the Error that 'g' packet message reply is too long. This is
because the buf_len calculated in the init_remote_state function for
microblaze target is based On the sizeof_g_packet and remote_packet_size
and the memory_packet_config->size. The sizeof_g_packet is 236 because the
number of reg num is 59 and 2* sizeof_g_packet comes to 472 .With shr and
shl entry and the buf_len is 472. This does not match the greater than
conditional statement  and works fine. Without shr and shl entry,the
sizeof_g_packets comes to 57*4 *2 = 456.  This doesn't match the criteria
in the process_g_packet function  leading to throwing of error message as
" 'g' packet message reply is too long".

ChangeLog:
2014-05-20 Ajit Agarwal <ajitkum@xilinx.com>

	* gdb/gdbserver/Makefile.in (microblaze-linux.c): New rule.

	* gdb/microblaze-tdep.c (microblaze_register_names): Added
	the rshr and rslr register names.

	* gdb/microblaze-tdep.h (microblaze_reg_num): Addition of
	field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM.
	(microblaze_frame_cache): Change in the index of
	register_offsets.

	* gdb/regformats/reg-microblaze.dat: New Register data file.

Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
---
 gdb/gdbserver/Makefile.in         |    5 +++-
 gdb/microblaze-tdep.c             |    3 +-
 gdb/microblaze-tdep.h             |    6 +++-
 gdb/regformats/reg-microblaze.dat |   41 +++++++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 4 deletions(-)
 create mode 100644 gdb/regformats/reg-microblaze.dat

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f773fa2..2454003 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -348,7 +348,8 @@ clean:
 	rm -f amd64-avx.c amd64-avx-linux.c
 	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 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
@@ -614,6 +615,8 @@ reg-cf.c : $(srcdir)/../regformats/reg-cf.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/reg-cf.dat reg-cf.c
 mips-linux.c : $(srcdir)/../regformats/mips-linux.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/mips-linux.dat mips-linux.c
+microblaze-linux.c : $(srcdir)/../regformats/reg-microblaze.dat $(regdat_sh)
+	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/reg-microblaze.dat mips-linux.c
 mips-dsp-linux.c : $(srcdir)/../regformats/mips-dsp-linux.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/mips-dsp-linux.dat mips-dsp-linux.c
 mips64-linux.c : $(srcdir)/../regformats/mips64-linux.dat $(regdat_sh)
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 14c1b52..4d63909 100644
--- a/gdb/microblaze-tdep.c
+++ b/gdb/microblaze-tdep.c
@@ -73,7 +73,8 @@ static const char *microblaze_register_names[] =
   "rpc",  "rmsr", "rear", "resr", "rfsr", "rbtr",
   "rpvr0", "rpvr1", "rpvr2", "rpvr3", "rpvr4", "rpvr5", "rpvr6",
   "rpvr7", "rpvr8", "rpvr9", "rpvr10", "rpvr11",
-  "redr", "rpid", "rzpr", "rtlbx", "rtlbsx", "rtlblo", "rtlbhi"
+  "redr", "rpid", "rzpr", "rtlbx", "rtlbsx", "rtlblo", "rtlbhi",
+  "rslr", "rshr"
 };
 
 #define MICROBLAZE_NUM_REGS ARRAY_SIZE (microblaze_register_names)
diff --git a/gdb/microblaze-tdep.h b/gdb/microblaze-tdep.h
index a532092..f7acfd6 100644
--- a/gdb/microblaze-tdep.h
+++ b/gdb/microblaze-tdep.h
@@ -42,7 +42,7 @@ struct microblaze_frame_cache
   int fp_regnum;
 
   /* Offsets to saved registers.  */
-  int register_offsets[57];	/* Must match MICROBLAZE_NUM_REGS.  */
+  int register_offsets[59];	/* Must match MICROBLAZE_NUM_REGS.  */
 
   /* Table of saved registers.  */
   struct trad_frame_saved_reg *saved_regs;
@@ -107,7 +107,9 @@ enum microblaze_regnum
   MICROBLAZE_RTLBX_REGNUM,
   MICROBLAZE_RTLBSX_REGNUM,
   MICROBLAZE_RTLBLO_REGNUM,
-  MICROBLAZE_RTLBHI_REGNUM
+  MICROBLAZE_RTLBHI_REGNUM,
+  MICROBLAZE_SLR_REGNUM,
+  MICROBLAZE_SHR_REGNUM
 };
 
 /* All registers are 32 bits.  */
diff --git a/gdb/regformats/reg-microblaze.dat b/gdb/regformats/reg-microblaze.dat
new file mode 100644
index 0000000..936bc44
--- /dev/null
+++ b/gdb/regformats/reg-microblaze.dat
@@ -0,0 +1,41 @@
+name:microblaze 
+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:msr 
+32:ear 
+32:esr 
+32:fsr 
+32:slr
+32:shr
-- 
1.7.1