[Patch, microblaze]: Add slr and shr regs

[Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

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

  Hello Michael:

  Based on the feedback, resubmitting the patch once again with all your replies and suggestions.

    [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

I am confused with your comments whereas it seems I have answered all queries(including yours) and incorporated your review comments. 
Just to make sure I repeat, see below answers to your queries.

>>Make sure you address my comments and incorporate my suggestions as well.

I made sure that the description is not too long at the same time it gives the complete picture. Hope this addresses your comments.

>>>I asked what is running on the target which is returning a different sized G packet.

It is  the gdbserver which is checking for the buf_len and 2* size_of_g_packet  in process_g_packet function which is not matching 
and returning with error mentioning that 'g' packet is too long. The G Packet initialization is done in init_remote_state function which 
is all happening when tar remote machine:1234 command is used.
Hope this clarifies as this error is nothing to do with what is running on target. On target the XMD is running.

Thanks & Regards
Ajit


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

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 05/22/14 23:53, Ajit Kumar Agarwal wrote:
>    Hello Michael:
>
>    Based on the feedback, resubmitting the patch once again with all your replies and suggestions.
>
>      [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
>
> I am confused with your comments whereas it seems I have answered all queries(including yours) and incorporated your review comments.
> Just to make sure I repeat, see below answers to your queries.
>
>>> Make sure you address my comments and incorporate my suggestions as well.
>
> I made sure that the description is not too long at the same time it gives the complete picture. Hope this addresses your comments.
>
>>>> I asked what is running on the target which is returning a different sized G packet.
>
> It is  the gdbserver which is checking for the buf_len and 2* size_of_g_packet  in process_g_packet function which is not matching
> and returning with error mentioning that 'g' packet is too long. The G Packet initialization is done in init_remote_state function which
> is all happening when tar remote machine:1234 command is used.

I'm quite familiar with how G packets are handled.  An explanation of this
is not answering my questions.

> Hope this clarifies as this error is nothing to do with what is running on target. On target the XMD is running.

I asked you to start over and resubmit a patch, including Changelog,
without the replies.  At this point, I don't know what Changelog corresponds
to the patch.

I asked if XMD was connected to the target.  You said it wasn't.  Now you say
that it is, and that gdbserver is running.  This doesn't make sense to me,
since if you are running gdbserver on the target, and it is built from the
same sources, you will never get a G packet mismatch.

Let's try this one more time:  GDB is connected to something when you
tell it to connect to the target with the "target remote" command.
This is returning a different sized G packet.  What is it that you are
connecting to?

>


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

Re: [Patch, microblaze]: Add slr and shr regs

[Yao Qi]

On 05/23/2014 02:53 PM, Ajit Kumar Agarwal wrote:
> +microblaze-linux.c : $(srcdir)/../regformats/reg-microblaze.dat $(regdat_sh)
> +	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/reg-microblaze.dat mips-linux.c
                                                                         ^^^^^^^^^^^^
Typo.

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

The more important thing is microblaze-linux.c and reg-microblaze.dat
should be generated by some .xml file in gdb/features/ directory.  If
that is intended, please add the .xml files, and generate the .c file
and .dat file in regformats/ directory.

-- 
Yao (齐尧)

Re: [Patch, microblaze]: Add slr and shr regs

[Pedro Alves]

On 05/23/2014 07:53 AM, Ajit Kumar Agarwal wrote:

> I am confused with your comments whereas it seems I have answered all queries(including yours) and incorporated your review comments. 

Doesn't look like mine have been addressed though.  ;-)

https://sourceware.org/ml/gdb-patches/2014-05/msg00481.html

-- 
Pedro Alves

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

On 05/22/14 23:53, Ajit Kumar Agarwal wrote:
>    Hello Michael:
>
>    Based on the feedback, resubmitting the patch once again with all your replies and suggestions.
>
>      [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
>
> I am confused with your comments whereas it seems I have answered all queries(including yours) and incorporated your review comments.
> Just to make sure I repeat, see below answers to your queries.
>
>>> Make sure you address my comments and incorporate my suggestions as well.
>
> I made sure that the description is not too long at the same time it gives the complete picture. Hope this addresses your comments.
>
>>>> I asked what is running on the target which is returning a different sized G packet.
>
> It is  the gdbserver which is checking for the buf_len and 2* 
> size_of_g_packet  in process_g_packet function which is not matching 
> and returning with error mentioning that 'g' packet is too long. The G Packet initialization is done in init_remote_state function which is all happening when tar remote machine:1234 command is used.

I'm quite familiar with how G packets are handled.  An explanation of this is not answering my questions.

> Hope this clarifies as this error is nothing to do with what is running on target. On target the XMD is running.

I asked you to start over and resubmit a patch, including Changelog, without the replies.  At this point, I don't know what Changelog corresponds to the patch.

I asked if XMD was connected to the target.  You said it wasn't.  Now you say that it is, and that gdbserver is running.  This doesn't make sense to me, since if you are running gdbserver on the target, and it is built from the same sources, you will never get a G packet mismatch.

>>Let's try this one more time:  GDB is connected to something when you tell it to connect to the target with the "target remote" command.
>>This is returning a different sized G packet.  What is it that you are connecting to?

Here is the flow from GBD Client to XMD that's what is happening:

1. XMD connects to the hardware target through JTAG.
2. XMD Opens a GDB Server on the local host 1234.
3. GDB Client will connect to  the host:1234 through TCP, when the command "tar remote localhost:1234" is given.

Thanks & Regards
Ajit 


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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Yao Qi [mailto:yao@codesourcery.com] 
Sent: Friday, May 23, 2014 2:00 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Joel Brobecker; Michael Eager; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 05/23/2014 02:53 PM, Ajit Kumar Agarwal wrote:
> +microblaze-linux.c : $(srcdir)/../regformats/reg-microblaze.dat $(regdat_sh)
> +	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/reg-microblaze.dat 
> +mips-linux.c
                                                                         ^^^^^^^^^^^^ Typo.

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

>>The more important thing is microblaze-linux.c and reg-microblaze.dat should be generated by some .xml file in gdb/features/ directory.  If that is intended, please add the .xml files, >>and generate the .c file and .dat file in regformats/ directory.

I will incorporate the above changes.

Thanks & Regards
Ajit
--
Yao (齐尧)

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Friday, May 23, 2014 2:14 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Joel Brobecker; Michael Eager; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 05/23/2014 07:53 AM, Ajit Kumar Agarwal wrote:

> I am confused with your comments whereas it seems I have answered all queries(including yours) and incorporated your review comments. 

>>Doesn't look like mine have been addressed though.  ;-)

>>https://sourceware.org/ml/gdb-patches/2014-05/msg00481.html

I will address your review comments.

Thanks & Regards
Ajit
-- 
Pedro Alves

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 05/23/14 02:46, Ajit Kumar Agarwal wrote:
>
>
>>> Let's try this one more time:  GDB is connected to something when you tell it to connect to the target with the "target remote" command.
>>> This is returning a different sized G packet.  What is it that you are connecting to?
>
> Here is the flow from GBD Client to XMD that's what is happening:
>
> 1. XMD connects to the hardware target through JTAG.
> 2. XMD Opens a GDB Server on the local host 1234.
> 3. GDB Client will connect to  the host:1234 through TCP, when the command "tar remote localhost:1234" is given.

Ajit --

Let me suggest this description of the problem:
   Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
   response to GDB's G request.  Starting with version x.xx, XMD added the
   slr and shr register, for a count of 59 registers.  This patch adds
   these registers to the expected G response.

Please resubmit the patch, in full, with ChangeLog.


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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

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


Based on the feedback and incorporated all review comment, updated the patch.

 [Patch, microblaze]: Add slr and shr regs

   Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
   response to GDB's G request.  Starting with version x.xx, XMD added the
   slr and shr register, for a count of 59 registers.  This patch adds
   these registers to the expected G response

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

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

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

        * microblaze-tdep.h (microblaze_reg_num): Add
        field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
        MICROBLAZE_NUM_REGS.
        (microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.

        * features/microblaze-cpu.xml: New file.

        * features/microblaze-linux.c: New file.

        * features/microblaze-linux.xml: New file.

        * regformats/reg-microblaze.dat: New file.

        * features/Makefile (microblaze-linux): Add
        microblaze-linux and microblaze-expedite.

Signed-off-by:Ajit Agarwal ajitkum@xilinx.com

Thanks & Regards
Ajit
-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Saturday, May 24, 2014 4:13 AM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 05/23/14 02:46, Ajit Kumar Agarwal wrote:
>
>
>>> Let's try this one more time:  GDB is connected to something when you tell it to connect to the target with the "target remote" command.
>>> This is returning a different sized G packet.  What is it that you are connecting to?
>
> Here is the flow from GBD Client to XMD that's what is happening:
>
> 1. XMD connects to the hardware target through JTAG.
> 2. XMD Opens a GDB Server on the local host 1234.
> 3. GDB Client will connect to  the host:1234 through TCP, when the command "tar remote localhost:1234" is given.

Ajit --

Let me suggest this description of the problem:
   Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
   response to GDB's G request.  Starting with version x.xx, XMD added the
   slr and shr register, for a count of 59 registers.  This patch adds
   these registers to the expected G response.

Please resubmit the patch, in full, with ChangeLog.


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

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

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

   Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
   response to GDB's G request.  Starting with version x.xx, XMD added the
   slr and shr register, for a count of 59 registers.  This patch adds
   these registers to the expected G response

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

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

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

	* microblaze-tdep.h (microblaze_reg_num): Add
	field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
	MICROBLAZE_NUM_REGS.
	(microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.

	* features/microblaze-cpu.xml: New file.

	* features/microblaze-linux.c: New file.

	* features/microblaze-linux.xml: New file.

	* regformats/reg-microblaze.dat: New file.

	* features/Makefile (microblaze-linux): Add
	microblaze-linux and microblaze-expedite.

Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
---
 gdb/features/Makefile             |    2 +
 gdb/features/microblaze-cpu.xml   |   49 ++++++++++++++++++++++++++++++
 gdb/features/microblaze-linux.c   |   60 +++++++++++++++++++++++++++++++++++++
 gdb/features/microblaze-linux.xml |   13 ++++++++
 gdb/gdbserver/Makefile.in         |    5 ++-
 gdb/microblaze-tdep.c             |    3 +-
 gdb/microblaze-tdep.h             |   48 +++++++++++++++--------------
 gdb/regformats/reg-microblaze.dat |   42 ++++++++++++++++++++++++++
 8 files changed, 197 insertions(+), 25 deletions(-)
 create mode 100644 gdb/features/microblaze-cpu.xml
 create mode 100644 gdb/features/microblaze-linux.c
 create mode 100644 gdb/features/microblaze-linux.xml
 create mode 100644 gdb/regformats/reg-microblaze.dat

diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index dbf4963..5e9ea70 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -46,6 +46,7 @@ WHICH = aarch64 \
 	i386/x32-avx i386/x32-avx-linux \
 	i386/x32-avx512 i386/x32-avx512-linux \
 	mips-linux mips-dsp-linux \
+	microblaze-linux \
 	mips64-linux mips64-dsp-linux \
 	nios2-linux \
 	rs6000/powerpc-32 \
@@ -90,6 +91,7 @@ mips-expedite = r29,pc
 mips-dsp-expedite = r29,pc
 mips64-expedite = r29,pc
 mips64-dsp-expedite = r29,pc
+microblaze-expedite = r1,pc
 nios2-linux-expedite = sp,pc
 powerpc-expedite = r1,pc
 rs6000/powerpc-cell32l-expedite = r1,pc,r0,orig_r3,r4
diff --git a/gdb/features/microblaze-cpu.xml b/gdb/features/microblaze-cpu.xml
new file mode 100644
index 0000000..7f2b5b4
--- /dev/null
+++ b/gdb/features/microblaze-cpu.xml
@@ -0,0 +1,49 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2007-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.cpu">
+  <reg name="r0" bitsize="32" regnum="0"/>
+  <reg name="r1" bitsize="32"/>
+  <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="msr" bitsize="32"/>
+  <reg name="ear" bitsize="32"/>
+  <reg name="esr" bitsize="32"/>
+  <reg name="fsr" bitsize="32"/>
+  <reg name="slr" bitsize="32"/>
+  <reg name="shr" bitsize="32"/>
+</feature>
diff --git a/gdb/features/microblaze-linux.c b/gdb/features/microblaze-linux.c
new file mode 100644
index 0000000..5923763
--- /dev/null
+++ b/gdb/features/microblaze-linux.c
@@ -0,0 +1,60 @@
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: microblaze-linux.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_microblaze_linux;
+static void
+initialize_tdesc_microblaze_linux (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+
+  set_tdesc_architecture (result, bfd_scan_arch ("MicroBlaze"));
+
+  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.MicroBlaze.cpu");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  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, "msr", 32, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "ear", 33, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "esr", 34, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "fsr", 35, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "slr", 36, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "shr", 37, 1, NULL, 32, "int");
+
+  tdesc_microblaze_linux = result;
+}
diff --git a/gdb/features/microblaze-linux.xml b/gdb/features/microblaze-linux.xml
new file mode 100644
index 0000000..2ae73e7
--- /dev/null
+++ b/gdb/features/microblaze-linux.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2007-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>
+  <architecture>MicroBlaze</architecture>
+  <osabi>GNU/Linux</osabi>
+  <xi:include href="microblaze-cpu.xml"/>
+</target>
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..8658bc5 100644
--- a/gdb/microblaze-tdep.h
+++ b/gdb/microblaze-tdep.h
@@ -26,28 +26,6 @@ struct gdbarch_tdep
 {
 };
 
-struct microblaze_frame_cache
-{
-  /* Base address.  */
-  CORE_ADDR base;
-  CORE_ADDR pc;
-
-  /* Do we have a frame?  */
-  int frameless_p;
-
-  /* Frame size.  */
-  int framesize;
-
-  /* Frame register.  */
-  int fp_regnum;
-
-  /* Offsets to saved registers.  */
-  int register_offsets[57];	/* Must match MICROBLAZE_NUM_REGS.  */
-
-  /* Table of saved registers.  */
-  struct trad_frame_saved_reg *saved_regs;
-};
-
 /* Register numbers.  */
 enum microblaze_regnum 
 {
@@ -107,9 +85,33 @@ enum microblaze_regnum
   MICROBLAZE_RTLBX_REGNUM,
   MICROBLAZE_RTLBSX_REGNUM,
   MICROBLAZE_RTLBLO_REGNUM,
-  MICROBLAZE_RTLBHI_REGNUM
+  MICROBLAZE_RTLBHI_REGNUM,
+  MICROBLAZE_SLR_REGNUM,
+  MICROBLAZE_SHR_REGNUM,
+  MICROBLAZE_NUM_REGS
 };
 
+struct microblaze_frame_cache
+{
+  /* Base address.  */
+  CORE_ADDR base;
+  CORE_ADDR pc;
+
+  /* Do we have a frame?  */
+  int frameless_p;
+
+  /* Frame size.  */
+  int framesize;
+
+  /* Frame register.  */
+  int fp_regnum;
+
+  /* Offsets to saved registers.  */
+  int register_offsets[MICROBLAZE_NUM_REGS];    /* Must match MICROBLAZE_NUM_REGS.  */
+
+  /* Table of saved registers.  */
+  struct trad_frame_saved_reg *saved_regs;
+};
 /* All registers are 32 bits.  */
 #define MICROBLAZE_REGISTER_SIZE 4
 
diff --git a/gdb/regformats/reg-microblaze.dat b/gdb/regformats/reg-microblaze.dat
new file mode 100644
index 0000000..b3bad0a
--- /dev/null
+++ b/gdb/regformats/reg-microblaze.dat
@@ -0,0 +1,42 @@
+# DO NOT EDIT: generated from microblaze-linux.xml
+name:microblaze_linux
+xmltarget:microblaze-linux.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:msr
+32:ear
+32:esr
+32:fsr
+32:slr
+32:shr
-- 
1.7.1

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 05/26/14 03:04, Ajit Kumar Agarwal wrote:
>
> Based on the feedback and incorporated all review comment, updated the patch.
>
>   [Patch, microblaze]: Add slr and shr regs
>
>     Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
>     response to GDB's G request.  Starting with version x.xx, XMD added the
>     slr and shr register, for a count of 59 registers.  This patch adds
>     these registers to the expected G response

What versions?


What targets have you built and tested?


>
> ChangeLog:
> 2014-05-26 Ajit Agarwal <ajitkum@xilinx.com>
>
>          * Makefile.in (microblaze-linux.c): New rule.
>
>          * microblaze-tdep.c (microblaze_register_names): Add
>          the rshr and rslr register names.
>
>          * microblaze-tdep.h (microblaze_reg_num): Add
>          field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
>          MICROBLAZE_NUM_REGS.
>          (microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.
>
>          * features/microblaze-cpu.xml: New file.
>
>          * features/microblaze-linux.c: New file.
>
>          * features/microblaze-linux.xml: New file.
>
>          * regformats/reg-microblaze.dat: New file.
>
>          * features/Makefile (microblaze-linux): Add
>          microblaze-linux and microblaze-expedite.
>
> Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
>
> Thanks & Regards
> Ajit
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagercon.com]
> Sent: Saturday, May 24, 2014 4:13 AM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 05/23/14 02:46, Ajit Kumar Agarwal wrote:
>>
>>
>>>> Let's try this one more time:  GDB is connected to something when you tell it to connect to the target with the "target remote" command.
>>>> This is returning a different sized G packet.  What is it that you are connecting to?
>>
>> Here is the flow from GBD Client to XMD that's what is happening:
>>
>> 1. XMD connects to the hardware target through JTAG.
>> 2. XMD Opens a GDB Server on the local host 1234.
>> 3. GDB Client will connect to  the host:1234 through TCP, when the command "tar remote localhost:1234" is given.
>
> Ajit --
>
> Let me suggest this description of the problem:
>     Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
>     response to GDB's G request.  Starting with version x.xx, XMD added the
>     slr and shr register, for a count of 59 registers.  This patch adds
>     these registers to the expected G response.
>
> Please resubmit the patch, in full, with ChangeLog.
>
>


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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com] 
Sent: Tuesday, May 27, 2014 12:05 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org; Yao Qi; Pedro Alves
Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 05/26/14 03:04, Ajit Kumar Agarwal wrote:
>
> Based on the feedback and incorporated all review comment, updated the patch.
>
>   [Patch, microblaze]: Add slr and shr regs
>
>     Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
>     response to GDB's G request.  Starting with version x.xx, XMD added the
>     slr and shr register, for a count of 59 registers.  This patch adds
>     these registers to the expected G response

>>What versions?

Update the problem description and here it is.

    [Patch, microblaze]: Add slr and shr regs
    
       Prior to version 2013.1, XMD's gdbserver stub returned 57 registers in
       response to GDB's G request.  Starting with version 2013.1, XMD added the
       slr and shr register, for a count of 59 registers.  This patch adds
       these registers to the expected G response.

>>What targets have you built and tested?

Microblaze target has been built and tested.

>
> ChangeLog:
> 2014-05-26 Ajit Agarwal <ajitkum@xilinx.com>
>
>          * Makefile.in (microblaze-linux.c): New rule.
>
>          * microblaze-tdep.c (microblaze_register_names): Add
>          the rshr and rslr register names.
>
>          * microblaze-tdep.h (microblaze_reg_num): Add
>          field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
>          MICROBLAZE_NUM_REGS.
>          (microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.
>
>          * features/microblaze-cpu.xml: New file.
>
>          * features/microblaze-linux.c: New file.
>
>          * features/microblaze-linux.xml: New file.
>
>          * regformats/reg-microblaze.dat: New file.
>
>          * features/Makefile (microblaze-linux): Add
>          microblaze-linux and microblaze-expedite.
>
> Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
>
> Thanks & Regards
> Ajit
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagercon.com]
> Sent: Saturday, May 24, 2014 4:13 AM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 05/23/14 02:46, Ajit Kumar Agarwal wrote:
>>
>>
>>>> Let's try this one more time:  GDB is connected to something when you tell it to connect to the target with the "target remote" command.
>>>> This is returning a different sized G packet.  What is it that you are connecting to?
>>
>> Here is the flow from GBD Client to XMD that's what is happening:
>>
>> 1. XMD connects to the hardware target through JTAG.
>> 2. XMD Opens a GDB Server on the local host 1234.
>> 3. GDB Client will connect to  the host:1234 through TCP, when the command "tar remote localhost:1234" is given.
>
> Ajit --
>
> Let me suggest this description of the problem:
>     Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
>     response to GDB's G request.  Starting with version x.xx, XMD added the
>     slr and shr register, for a count of 59 registers.  This patch adds
>     these registers to the expected G response.
>
> Please resubmit the patch, in full, with ChangeLog.
>
>


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

Re: [Patch, microblaze]: Add slr and shr regs

[Pedro Alves]

On 05/27/2014 08:46 AM, Ajit Kumar Agarwal wrote:

> Update the problem description and here it is.
> 
>     [Patch, microblaze]: Add slr and shr regs
>     
>        Prior to version 2013.1, XMD's gdbserver stub returned 57 registers in
>        response to GDB's G request.  Starting with version 2013.1, XMD added the
>        slr and shr register, for a count of 59 registers.  This patch adds
>        these registers to the expected G response.
> 
>>> What targets have you built and tested?
> 
> Microblaze target has been built and tested.

Did you test new GDB against old stub?

I've looked at the microblaze reference guide, and saw that these
registers are optional.  What happens if you debug a system where
C_USE_STACK_PROTECTION was set to 0?  Do we still show these
registers to the user?

-- 
Pedro Alves

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 05/27/14 00:46, Ajit Kumar Agarwal wrote:
>
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagercon.com]
> Sent: Tuesday, May 27, 2014 12:05 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org; Yao Qi; Pedro Alves
> Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 05/26/14 03:04, Ajit Kumar Agarwal wrote:
>>
>> Based on the feedback and incorporated all review comment, updated the patch.
>>
>>    [Patch, microblaze]: Add slr and shr regs
>>
>>      Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
>>      response to GDB's G request.  Starting with version x.xx, XMD added the
>>      slr and shr register, for a count of 59 registers.  This patch adds
>>      these registers to the expected G response
>
>>> What versions?
>
> Update the problem description and here it is.
>
>      [Patch, microblaze]: Add slr and shr regs
>
>         Prior to version 2013.1, XMD's gdbserver stub returned 57 registers in
>         response to GDB's G request.  Starting with version 2013.1, XMD added the
>         slr and shr register, for a count of 59 registers.  This patch adds
>         these registers to the expected G response.

I'm not familiar with this version numbering.  Is this an internal number?
Is this displayed by XMD?  Which version of EDK does this correspond to?

>
>>> What targets have you built and tested?
>
> Microblaze target has been built and tested.

Exactly which --target triples were built and tested?
Please make sure that --target=microblaze-xilinx-elf
has been built and tested.

>
>>
>> ChangeLog:
>> 2014-05-26 Ajit Agarwal <ajitkum@xilinx.com>
>>
>>           * Makefile.in (microblaze-linux.c): New rule.
>>
>>           * microblaze-tdep.c (microblaze_register_names): Add
>>           the rshr and rslr register names.
>>
>>           * microblaze-tdep.h (microblaze_reg_num): Add
>>           field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
>>           MICROBLAZE_NUM_REGS.
>>           (microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.
>>
>>           * features/microblaze-cpu.xml: New file.
>>
>>           * features/microblaze-linux.c: New file.
>>
>>           * features/microblaze-linux.xml: New file.
>>
>>           * regformats/reg-microblaze.dat: New file.
>>
>>           * features/Makefile (microblaze-linux): Add
>>           microblaze-linux and microblaze-expedite.
>>
>> Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
>>
>> Thanks & Regards
>> Ajit
>> -----Original Message-----
>> From: Michael Eager [mailto:eager@eagercon.com]
>> Sent: Saturday, May 24, 2014 4:13 AM
>> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
>> Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
>> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>>
>> On 05/23/14 02:46, Ajit Kumar Agarwal wrote:
>>>
>>>
>>>>> Let's try this one more time:  GDB is connected to something when you tell it to connect to the target with the "target remote" command.
>>>>> This is returning a different sized G packet.  What is it that you are connecting to?
>>>
>>> Here is the flow from GBD Client to XMD that's what is happening:
>>>
>>> 1. XMD connects to the hardware target through JTAG.
>>> 2. XMD Opens a GDB Server on the local host 1234.
>>> 3. GDB Client will connect to  the host:1234 through TCP, when the command "tar remote localhost:1234" is given.
>>
>> Ajit --
>>
>> Let me suggest this description of the problem:
>>      Prior to version x.xx, XMD's gdbserver stub returned 57 registers in
>>      response to GDB's G request.  Starting with version x.xx, XMD added the
>>      slr and shr register, for a count of 59 registers.  This patch adds
>>      these registers to the expected G response.
>>
>> Please resubmit the patch, in full, with ChangeLog.
>>
>>
>
>


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

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 05/27/14 00:46, Ajit Kumar Agarwal wrote:
>
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagercon.com]
> Sent: Tuesday, May 27, 2014 12:05 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org; Yao Qi; Pedro Alves
> Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 05/26/14 03:04, Ajit Kumar Agarwal wrote:
>>
>> Based on the feedback and incorporated all review comment, updated the patch.

Hi Ajit --

I've reviewed your patch in more detail.

There are a number of issues which need to be addressed.


> Update the problem description and here it is.
>
>      [Patch, microblaze]: Add slr and shr regs
>
>         Prior to version 2013.1, XMD's gdbserver stub returned 57 registers in
>         response to GDB's G request.  Starting with version 2013.1, XMD added the
>         slr and shr register, for a count of 59 registers.  This patch adds
>         these registers to the expected G response.

XMD identifies itself with the version of EDK/SDK in which it was
released, not with the version number you mention.

I reviewed the MicroBlaze architecture documents for EDK 9.1i and EDK
14.2i.  It appears that at some time after EDK 9.1i, the MicroBlaze
processor architecture was extended to add two new stack protection
registers: slr (stack low register) and shr (stack high registers).
It isn't clear from the documentation I have at hand which MicroBlaze
processor version contained this architectural change.  EDK 9.1i mentions
versions through V6.00, while EDK 14.2i mentions versions from v7.30
through v8.40.

Evidently, as part of this architectural change, XMD's gdbserver stub
released with the corresponding EDK version was modified to return these
additional registers.

Your patch needs to be modified to support multiple versions of the
MicroBlaze architecture.  It is generally not acceptable to break
support for one processor version when you add support for a different
processor version.

Your patch should also mention that it changes behavior of gdbserver
to match XMD.

>> ChangeLog:
>> 2014-05-26 Ajit Agarwal <ajitkum@xilinx.com>
>>
>>           * Makefile.in (microblaze-linux.c): New rule.

There is no new Makefile rule in the patch.

>>           * microblaze-tdep.c (microblaze_register_names): Add
>>           the rshr and rslr register names.

This needs to be modified to support the MicroBlaze versions which
are currently supported (EDK 9.1i) as well as more recent versions.

>>           * microblaze-tdep.h (microblaze_reg_num): Add
>>           field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
>>           MICROBLAZE_NUM_REGS.
>>           (microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.

Same as above.

>>           * features/microblaze-cpu.xml: New file.
>>
>>           * features/microblaze-linux.c: New file.
>>
>>           * features/microblaze-linux.xml: New file.
>>
>>           * regformats/reg-microblaze.dat: New file.
>>
>>           * features/Makefile (microblaze-linux): Add
>>           microblaze-linux and microblaze-expedite.

I do not have any way to test microblaze-linux.  Make sure that
your patch works with both -target=microblaze-linux and
-target=microblaze-xilinx-gnu, and with both EDK 9.1i and 14.2i,
at a minimum.  Please provide documentation which show that
this patch works using gdbserver on a Linux target, as well
as using XMD.

When you resubmit the patch, please make sure that you
check for whitespace errors.  Make sure that you address all
of the questions and comments made here and by other reviewers.

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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

Sorry for the late response as I was on Vacation. Please find my response inlined below.

-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com] 
Sent: Thursday, June 05, 2014 9:24 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 05/27/14 00:46, Ajit Kumar Agarwal wrote:
>
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagercon.com]
> Sent: Tuesday, May 27, 2014 12:05 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org; Yao Qi; Pedro 
> Alves
> Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju 
> Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 05/26/14 03:04, Ajit Kumar Agarwal wrote:
>>
>> Based on the feedback and incorporated all review comment, updated the patch.

Hi Ajit --

I've reviewed your patch in more detail.

There are a number of issues which need to be addressed.


> Update the problem description and here it is.
>
>      [Patch, microblaze]: Add slr and shr regs
>


>         Prior to version 2013.1, XMD's gdbserver stub returned 57 registers in
>         response to GDB's G request.  Starting with version 2013.1, XMD added the
>         slr and shr register, for a count of 59 registers.  This patch adds
>         these registers to the expected G response.

>>XMD identifies itself with the version of EDK/SDK in which it was released, not with the version number you mention.

>>I reviewed the MicroBlaze architecture documents for EDK 9.1i and EDK 14.2i.  It appears that at some time after EDK 9.1i, the MicroBlaze processor architecture was extended to add >>two new stack protection
>>registers: slr (stack low register) and shr (stack high registers).
>>It isn't clear from the documentation I have at hand which MicroBlaze processor version contained this architectural change.  EDK 9.1i mentions versions through V6.00, while EDK 14.2i >>mentions versions from v7.30 through v8.40.

>>Evidently, as part of this architectural change, XMD's gdbserver stub released with the corresponding EDK version was modified to return these additional registers.

>>Your patch needs to be modified to support multiple versions of the MicroBlaze architecture.  It is generally not acceptable to break support for one processor version when you add >>support for a different processor version.

>>Your patch should also mention that it changes behavior of gdbserver to match XMD.

SLR/SHR was added in MicroBlaze v8.10.a, EDK 13.1.  XMD's gdbserver stub releasesed before this design throws an error mentioning gdb is not supported  for the version below v8.10.a, EDK 13.1.

The slr(stack low register) and shr(stack high registers) are implemented based on C_USE_STACK_PROTECTION is set 0/1. Microblaze being the reconfigurable architecture the design can be selected with and without these registers. Its hard to identify in gdb whether these registers is being implemented for the design or not. In XMD where the gdb client connects to the local host we always display the shr and shl registers irrespective of  C_USE_STACK_PROTECTION is set or not. In the case where the design is not implemented with these register we always display the content to be 0 or ? to the user.  gdb will also display these registers when the C_USE_STACK_PROTECTION is set 0/1. 

>> ChangeLog:
>> 2014-05-26 Ajit Agarwal <ajitkum@xilinx.com>
>>
>>           * Makefile.in (microblaze-linux.c): New rule.

There is no new Makefile rule in the patch.

>>           * microblaze-tdep.c (microblaze_register_names): Add
>>           the rshr and rslr register names.

This needs to be modified to support the MicroBlaze versions which are currently supported (EDK 9.1i) as well as more recent versions.

>>           * microblaze-tdep.h (microblaze_reg_num): Add
>>           field MICROBLAZE_SLR_REGNUM and MICROBLAZE_SHR_REGNUM
>>           MICROBLAZE_NUM_REGS.
>>           (microblaze_frame_cache): Use of MICROBLAZE_NUM_REGS.

Same as above.

>>           * features/microblaze-cpu.xml: New file.
>>
>>           * features/microblaze-linux.c: New file.
>>
>>           * features/microblaze-linux.xml: New file.
>>
>>           * regformats/reg-microblaze.dat: New file.
>>
>>           * features/Makefile (microblaze-linux): Add
>>           microblaze-linux and microblaze-expedite.

>>I do not have any way to test microblaze-linux.  Make sure that your patch works with both -target=microblaze-linux and -target=microblaze-xilinx-gnu, and with both EDK 9.1i and >>14.2i, at a minimum.  Please provide documentation which show that this patch works using gdbserver on a Linux target, as well as using XMD.

Same as the Explanation above. The native Linux support patch is not yet  submitted and we will submitting the patches after this patch.  XMD testing is done and it works fine.

When you resubmit the patch, please make sure that you check for whitespace errors.  Make sure that you address all of the questions and comments made here and by other reviewers.

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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

Sorry for the late response as I was on Vacation. Please find my response inlined below.



-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Pedro Alves
Sent: Tuesday, May 27, 2014 2:24 PM
To: Ajit Kumar Agarwal; Michael Eager; gdb-patches@sourceware.org; Yao Qi
Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 05/27/2014 08:46 AM, Ajit Kumar Agarwal wrote:

> Update the problem description and here it is.
> 
>     [Patch, microblaze]: Add slr and shr regs
>     
>        Prior to version 2013.1, XMD's gdbserver stub returned 57 registers in
>        response to GDB's G request.  Starting with version 2013.1, XMD added the
>        slr and shr register, for a count of 59 registers.  This patch adds
>        these registers to the expected G response.
> 
>>> What targets have you built and tested?
> 
> Microblaze target has been built and tested.

>>Did you test new GDB against old stub?

>>I've looked at the microblaze reference guide, and saw that these registers are optional.  What happens if you debug a system where C_USE_STACK_PROTECTION was set to 0?  Do we >>still show these registers to the user?

SLR/SHR was added in MicroBlaze v8.10.a, EDK 13.1.  XMD's gdbserver stub releasesed before this design throws an error mentioning gdb is not supported  for the version below v8.10.a, EDK 13.1.

The slr(stack low register) and shr(stack high registers) are implemented based on C_USE_STACK_PROTECTION is set 0/1. Microblaze being the reconfigurable architecture the design can be selected with and without these registers. Its hard to identify in gdb whether these registers is being implemented for the design or not. In XMD where the gdb client connects to the local host we always display the shr and shl registers irrespective of  C_USE_STACK_PROTECTION is set or not. In the case where the design is not implemented with these register we always display the content to be 0 or ? to the user.  gdb will also display these registers when the C_USE_STACK_PROTECTION is set 0/1. 


--
Pedro Alves

Re: [Patch, microblaze]: Add slr and shr regs

[Pedro Alves]

On 06/09/2014 06:26 PM, Ajit Kumar Agarwal wrote:
> The slr(stack low register) and shr(stack high registers) are implemented based
> on C_USE_STACK_PROTECTION is set 0/1. Microblaze being the reconfigurable architecture
> the design can be selected with and without these registers. Its hard to identify in gdb
> whether these registers is being implemented for the design or not. In XMD where the gdb
> client connects to the local host we always display the shr and shl registers irrespective
> of  C_USE_STACK_PROTECTION is set or not. In the case where the design is not
> implemented with these register we always display the content to be 0 or ? to the user.
> gdb will also display these registers when the C_USE_STACK_PROTECTION is set 0/1.

This is exactly what target descriptions are supposed to solve.  With those,
you can have the target tell GDB about any random register, and GDB will
know about it, without having to change GDB.  If the register in question
are an important group that GDB needs to be aware of them (seems to be the
case here), then target descriptions have this concept of "target features"
to address it.  Grep for tdesc_find_feature in the source tree for numerous
examples, and see the "Target Description" and "Standard Target Features" in
the manual.

-- 
Pedro Alves

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 06/09/14 10:26, Ajit Kumar Agarwal wrote:
>
> Sorry for the late response as I was on Vacation. Please find my response inlined below.

It's difficult to figure out which are your responses and which
is text quoted from previous email.

AJA:
> SLR/SHR was added in MicroBlaze v8.10.a, EDK 13.1.  XMD's gdbserver stub releasesed before this design throws an error mentioning gdb is not supported  for the version below v8.10.a, EDK 13.1.

I don't know what this means.  XMD released with EDK 9.1i doesn't generate any
such error message and works with mb-gdb built from FSF sources.

There are several ways to make gdb to adapt to different target architectures.
Pedro described the preferred solution, but there are others.

MJE:
>>> I do not have any way to test microblaze-linux.  Make sure that your patch works with both -target=microblaze-linux and -target=microblaze-xilinx-gnu, and with both EDK 9.1i and >>14.2i, at a minimum.  Please provide documentation which show that this patch works using gdbserver on a Linux target, as well as using XMD.

AJA:
> Same as the Explanation above. The native Linux support patch is not yet  submitted and we will submitting the patches after this patch.  XMD testing is done and it works fine.

This isn't responsive to my request.

The only way that your patch makes sense is if you build
gdbserver for a microblaze-linux target.  If this is not
what you are doing, you need to explain (as previously
requested) how you are building and testing the patch.

MJE:
> When you resubmit the patch, please make sure that you check for whitespace errors.  Make sure that you address all of the questions and comments made here and by other reviewers.

Please respond to all comments.

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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com] 
Sent: Monday, June 09, 2014 11:59 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 06/09/14 10:26, Ajit Kumar Agarwal wrote:
>
> Sorry for the late response as I was on Vacation. Please find my response inlined below.

It's difficult to figure out which are your responses and which is text quoted from previous email.

AJA:
> SLR/SHR was added in MicroBlaze v8.10.a, EDK 13.1.  XMD's gdbserver stub releasesed before this design throws an error mentioning gdb is not supported  for the version below v8.10.a, EDK 13.1.

I don't know what this means.  XMD released with EDK 9.1i doesn't generate any such error message and works with mb-gdb built from FSF sources.

Ajit:   The necessity of the XMD to generate such error message came when we have shr and shl support in Microblaze v8.10.a, EDK 13.1  to have downward compatibility. The XMD you are using is quite old.

There are several ways to make gdb to adapt to different target architectures.
Pedro described the preferred solution, but there are others.

Ajit: We would surely like to implement as suggested by Pedro.

MJE:
>>> I do not have any way to test microblaze-linux.  Make sure that your patch works with both -target=microblaze-linux and -target=microblaze-xilinx-gnu, and with both EDK 9.1i and >>14.2i, at a minimum.  Please provide documentation which show that this patch works using gdbserver on a Linux target, as well as using XMD.

AJA:
> Same as the Explanation above. The native Linux support patch is not yet  submitted and we will submitting the patches after this patch.  XMD testing is done and it works fine.

This isn't responsive to my request.

The only way that your patch makes sense is if you build gdbserver for a microblaze-linux target.  If this is not what you are doing, you need to explain (as previously
requested) how you are building and testing the patch.

Ajit:  I might have misunderstood the question. We build the  binutils which comes with gdb with target as microblaze-xilinx-elf.  We do the following,
1. Invoke the XMD with gdbserver on.
2. XMD connects to the target.
3. Invoke mb-gdb with the executables to debug.
4.Invoke command " tar remote localhost:1234".
5. Invoke the command "load"
6. Set the breakpoints and do the normal debugging.

Kindly be more explicit regarding this question.

MJE:
> When you resubmit the patch, please make sure that you check for whitespace errors.  Make sure that you address all of the questions and comments made here and by other reviewers.

Please respond to all comments.

Ajit: Sure I will addressed all the comments.


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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Monday, June 09, 2014 11:27 PM
To: Ajit Kumar Agarwal; Michael Eager; gdb-patches@sourceware.org; Yao Qi
Cc: Joel Brobecker; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 06/09/2014 06:26 PM, Ajit Kumar Agarwal wrote:
> The slr(stack low register) and shr(stack high registers) are 
> implemented based on C_USE_STACK_PROTECTION is set 0/1. Microblaze 
> being the reconfigurable architecture the design can be selected with 
> and without these registers. Its hard to identify in gdb whether these 
> registers is being implemented for the design or not. In XMD where the 
> gdb client connects to the local host we always display the shr and 
> shl registers irrespective of  C_USE_STACK_PROTECTION is set or not. In the case where the design is not implemented with these register we always display the content to be 0 or ? to the user.
> gdb will also display these registers when the C_USE_STACK_PROTECTION is set 0/1.

This is exactly what target descriptions are supposed to solve.  With those, you can have the target tell GDB about any random register, and GDB will know about it, without having to change GDB.  If the register in question are an important group that GDB needs to be aware of them (seems to be the case here), then target descriptions have this concept of "target features"
to address it.  Grep for tdesc_find_feature in the source tree for numerous examples, and see the "Target Description" and "Standard Target Features" in the manual.

Ajit: Thanks Pedro!! We would surely like to implement this feature in gdb for Microblaze.

--
Pedro Alves

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 06/09/14 12:31, Ajit Kumar Agarwal wrote:
>
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Monday, June 09, 2014 11:59 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 06/09/14 10:26, Ajit Kumar Agarwal wrote:
>>
>> Sorry for the late response as I was on Vacation. Please find my response inlined below.
>
> It's difficult to figure out which are your responses and which is text quoted from previous email.
>
> AJA:
>> SLR/SHR was added in MicroBlaze v8.10.a, EDK 13.1.  XMD's gdbserver stub releasesed before this design throws an error mentioning gdb is not supported  for the version below v8.10.a, EDK 13.1.
>
> I don't know what this means.  XMD released with EDK 9.1i doesn't generate any such error message and works with mb-gdb built from FSF sources.
>
> Ajit:   The necessity of the XMD to generate such error message came when we have shr and shl support in Microblaze v8.10.a, EDK 13.1  to have downward compatibility. The XMD you are using is quite old.
>
> There are several ways to make gdb to adapt to different target architectures.
> Pedro described the preferred solution, but there are others.
>
> Ajit: We would surely like to implement as suggested by Pedro.
>
> MJE:
>>>> I do not have any way to test microblaze-linux.  Make sure that your patch works with both -target=microblaze-linux and -target=microblaze-xilinx-gnu, and with both EDK 9.1i and >>14.2i, at a minimum.  Please provide documentation which show that this patch works using gdbserver on a Linux target, as well as using XMD.
>
> AJA:
>> Same as the Explanation above. The native Linux support patch is not yet  submitted and we will submitting the patches after this patch.  XMD testing is done and it works fine.
>
> This isn't responsive to my request.
>
> The only way that your patch makes sense is if you build gdbserver for a microblaze-linux target.  If this is not what you are doing, you need to explain (as previously
> requested) how you are building and testing the patch.
>
> Ajit:  I might have misunderstood the question. We build the  binutils which comes with gdb with target as microblaze-xilinx-elf.  We do the following,
> 1. Invoke the XMD with gdbserver on.

XMD is not running gdbserver built with these sources, is it?

Your patch modified files under gdb/gdbserver.  When you
build binutils/gdb with -target=microblaze-xilinx-elf, these
files are not built.

It would have helped if you previously answered my question
about what targets you were building by saying
-target=microblaze-xilinx-elf, rather than just "Microblaze target".

> 2. XMD connects to the target.
> 3. Invoke mb-gdb with the executables to debug.
> 4.Invoke command " tar remote localhost:1234".
> 5. Invoke the command "load"
> 6. Set the breakpoints and do the normal debugging.
>
> Kindly be more explicit regarding this question.
>
> MJE:
>> When you resubmit the patch, please make sure that you check for whitespace errors.  Make sure that you address all of the questions and comments made here and by other reviewers.
>
> Please respond to all comments.
>
> Ajit: Sure I will addressed all the comments.
>
>


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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com] 
Sent: Tuesday, June 10, 2014 1:37 AM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 06/09/14 12:31, Ajit Kumar Agarwal wrote:
>
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Monday, June 09, 2014 11:59 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 06/09/14 10:26, Ajit Kumar Agarwal wrote:
>>
>> Sorry for the late response as I was on Vacation. Please find my response inlined below.
>
> It's difficult to figure out which are your responses and which is text quoted from previous email.
>
> AJA:
>> SLR/SHR was added in MicroBlaze v8.10.a, EDK 13.1.  XMD's gdbserver stub releasesed before this design throws an error mentioning gdb is not supported  for the version below v8.10.a, EDK 13.1.
>
> I don't know what this means.  XMD released with EDK 9.1i doesn't generate any such error message and works with mb-gdb built from FSF sources.
>
> Ajit:   The necessity of the XMD to generate such error message came when we have shr and shl support in Microblaze v8.10.a, EDK 13.1  to have downward compatibility. The XMD you are using is quite old.
>
> There are several ways to make gdb to adapt to different target architectures.
> Pedro described the preferred solution, but there are others.
>
> Ajit: We would surely like to implement as suggested by Pedro.
>
> MJE:
>>>> I do not have any way to test microblaze-linux.  Make sure that your patch works with both -target=microblaze-linux and -target=microblaze-xilinx-gnu, and with both EDK 9.1i and >>14.2i, at a minimum.  Please provide documentation which show that this patch works using gdbserver on a Linux target, as well as using XMD.
>
> AJA:
>> Same as the Explanation above. The native Linux support patch is not yet  submitted and we will submitting the patches after this patch.  XMD testing is done and it works fine.
>
> This isn't responsive to my request.
>
> The only way that your patch makes sense is if you build gdbserver for 
> a microblaze-linux target.  If this is not what you are doing, you 
> need to explain (as previously
> requested) how you are building and testing the patch.
>
> Ajit:  I might have misunderstood the question. We build the  binutils 
> which comes with gdb with target as microblaze-xilinx-elf.  We do the following, 1. Invoke the XMD with gdbserver on.

>>XMD is not running gdbserver built with these sources, is it?

Yes XMD is not running gdbserver built with these sources.

>>Your patch modified files under gdb/gdbserver.  When you build binutils/gdb with -target=microblaze-xilinx-elf, these files are not built.

The patch related to building gdbserver with the latest FSF Sources will be the next patch which I am going to  submit for review. 

>
>>It would have helped if you previously answered my question about what targets you were building by saying -target=microblaze-xilinx-elf, rather than just "Microblaze target".

Thanks for this!! 

> 2. XMD connects to the target.
> 3. Invoke mb-gdb with the executables to debug.
> 4.Invoke command " tar remote localhost:1234".
> 5. Invoke the command "load"
> 6. Set the breakpoints and do the normal debugging.
>
> Kindly be more explicit regarding this question.
>
> MJE:
>> When you resubmit the patch, please make sure that you check for whitespace errors.  Make sure that you address all of the questions and comments made here and by other reviewers.
>
> Please respond to all comments.
>
> Ajit: Sure I will addressed all the comments.
>
>


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

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 06/10/14 06:50, Ajit Kumar Agarwal wrote:
>
>>> XMD is not running gdbserver built with these sources, is it?
>
> Yes XMD is not running gdbserver built with these sources.
>
>>> Your patch modified files under gdb/gdbserver.  When you build binutils/gdb with -target=microblaze-xilinx-elf, these files are not built.
>
> The patch related to building gdbserver with the latest FSF Sources will be the next patch which I am going to  submit for review.

Changes related to building gdbserver should be submitted
together.  It sounds like this patch is dependent on some
future patch.

Please submit a revised patch which addresses one problem:
support for the added registers in MicroBlaze v8.10a.  As
mentioned previously, gdb built with this patch should continue
to support the same versions of MicroBlaze that are currently
supported.

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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com]
Sent: Tuesday, June 10, 2014 7:42 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 06/10/14 06:50, Ajit Kumar Agarwal wrote:
>
>>> XMD is not running gdbserver built with these sources, is it?
>
> Yes XMD is not running gdbserver built with these sources.
>
>>> Your patch modified files under gdb/gdbserver.  When you build binutils/gdb with -target=microblaze-xilinx-elf, these files are not built.
>
> The patch related to building gdbserver with the latest FSF Sources will be the next patch which I am going to  submit for review.

>>Changes related to building gdbserver should be submitted together.  It sounds like this patch is dependent on some future patch.

Could you please explain why you think  building gdbserver should be submitted together?. This patch is related to the problem of Remote G Packet error which is mainly targeted for baremetal.
Solving the mismatching error between packets returning from XMD Stub and GDB.

This is plainly independent of the  patch that deals with gdbserver patch which mainly deals with native linux debugging support which is quite independent of this patch. 

>>Please submit a revised patch which addresses one problem:
>>support for the added registers in MicroBlaze v8.10a.  As mentioned previously, gdb built with this patch should continue to support the same versions of MicroBlaze that are currently supported.

Support for SHR/SHL registers with baremetal support is quite different from the patch with building gdbserver as XMD doesn’t use the gdbserver stub from the FSF sources. It has its own gdbserver And we don’t have plans to replace gdb Server in XMD with gdbserver from FSF Sources. The building of gdbServer  from FSF is mainly related to Linux native debugging and use for Linux image Support of MIcroblaze which is quite independent of the patch submitted for baremetal.

I don’t see any relations between the two and why the patches should be submitted together.
-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: [Patch, microblaze]: Add slr and shr regs

[Michael Eager]

On 06/10/14 07:49, Ajit Kumar Agarwal wrote:
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Tuesday, June 10, 2014 7:42 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 06/10/14 06:50, Ajit Kumar Agarwal wrote:
>>
>>>> XMD is not running gdbserver built with these sources, is it?
>>
>> Yes XMD is not running gdbserver built with these sources.
>>
>>>> Your patch modified files under gdb/gdbserver.  When you build binutils/gdb with -target=microblaze-xilinx-elf, these files are not built.
>>
>> The patch related to building gdbserver with the latest FSF Sources will be the next patch which I am going to  submit for review.
>
>>> Changes related to building gdbserver should be submitted together.  It sounds like this patch is dependent on some future patch.
>
> Could you please explain why you think  building gdbserver should be submitted together?. This patch is related to the problem of Remote G Packet error which is mainly targeted for baremetal.
> Solving the mismatching error between packets returning from XMD Stub and GDB.

Your patch contains changes to gdbserver.  As far as you have
indicated, you haven't built or tested gdbserver from these sources.

> This is plainly independent of the  patch that deals with gdbserver patch which mainly deals with native linux debugging support which is quite independent of this patch.

I have no idea what your future patch will contain.

I don't particularly care whether you submit the
gdbserver changes in this patch together with
this unknown future patch, or whether you submit them
as multiple patches.  It appears that these changes are
inappropriate for a patch which is unrelated.

>>> Please submit a revised patch which addresses one problem:
>>> support for the added registers in MicroBlaze v8.10a.  As mentioned previously, gdb built with this patch should continue to support the same versions of MicroBlaze that are currently supported.
>
> Support for SHR/SHL registers with baremetal support is quite different from the patch with building gdbserver as XMD doesn’t use the gdbserver stub from the FSF sources. It has its own gdbserver And we don’t have plans to replace gdb Server in XMD with gdbserver from FSF Sources. The building of gdbServer  from FSF is mainly related to Linux native debugging and use for Linux image Support of MIcroblaze which is quite independent of the patch submitted for baremetal.

Yes, support for baremetal is quite different from Linux patches.
This patch is only for baremetal support and should not contain
changes to gdbserver.

> I don’t see any relations between the two and why the patches should be submitted together.

I'm asking you to have one patch address one change.


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

RE: [Patch, microblaze]: Add slr and shr regs

[Ajit Kumar Agarwal]

Thank you  all. All the feedbacks have been incorporated and the fresh patch will be sent in the next mail.

-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com] 
Sent: Tuesday, June 10, 2014 8:57 PM
To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch, microblaze]: Add slr and shr regs

On 06/10/14 07:49, Ajit Kumar Agarwal wrote:
>
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Tuesday, June 10, 2014 7:42 PM
> To: Ajit Kumar Agarwal; gdb-patches@sourceware.org
> Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Add slr and shr regs
>
> On 06/10/14 06:50, Ajit Kumar Agarwal wrote:
>>
>>>> XMD is not running gdbserver built with these sources, is it?
>>
>> Yes XMD is not running gdbserver built with these sources.
>>
>>>> Your patch modified files under gdb/gdbserver.  When you build binutils/gdb with -target=microblaze-xilinx-elf, these files are not built.
>>
>> The patch related to building gdbserver with the latest FSF Sources will be the next patch which I am going to  submit for review.
>
>>> Changes related to building gdbserver should be submitted together.  It sounds like this patch is dependent on some future patch.
>
> Could you please explain why you think  building gdbserver should be submitted together?. This patch is related to the problem of Remote G Packet error which is mainly targeted for baremetal.
> Solving the mismatching error between packets returning from XMD Stub and GDB.

Your patch contains changes to gdbserver.  As far as you have indicated, you haven't built or tested gdbserver from these sources.

> This is plainly independent of the  patch that deals with gdbserver patch which mainly deals with native linux debugging support which is quite independent of this patch.

I have no idea what your future patch will contain.

I don't particularly care whether you submit the gdbserver changes in this patch together with this unknown future patch, or whether you submit them as multiple patches.  It appears that these changes are inappropriate for a patch which is unrelated.

>>> Please submit a revised patch which addresses one problem:
>>> support for the added registers in MicroBlaze v8.10a.  As mentioned previously, gdb built with this patch should continue to support the same versions of MicroBlaze that are currently supported.
>
> Support for SHR/SHL registers with baremetal support is quite different from the patch with building gdbserver as XMD doesn’t use the gdbserver stub from the FSF sources. It has its own gdbserver And we don’t have plans to replace gdb Server in XMD with gdbserver from FSF Sources. The building of gdbServer  from FSF is mainly related to Linux native debugging and use for Linux image Support of MIcroblaze which is quite independent of the patch submitted for baremetal.

Yes, support for baremetal is quite different from Linux patches.
This patch is only for baremetal support and should not contain changes to gdbserver.

> I don’t see any relations between the two and why the patches should be submitted together.

I'm asking you to have one patch address one change.


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