This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [Patch, microblaze]: Add slr and shr regs
- From: Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>
- To: Michael Eager <eager at eagerm dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Cc: Vinod Kathail <vinodk at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, Nagaraju Mekala <nmekala at xilinx dot com>
- Date: Mon, 9 Jun 2014 17:26:48 +0000
- Subject: RE: [Patch, microblaze]: Add slr and shr regs
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=pass (sender IP is 149.199.60.83) smtp dot mailfrom=ajit dot kumar dot agarwal at xilinx dot com;
- References: <c6fe468a-5e02-430a-b34b-73de6bfd2b97 at BY2FFO11FD021 dot protection dot gbl> <537EFA08 dot 1060309 at eagercon dot com> <f0c2c22a-d3d6-45d1-92ba-660dd364fd89 at BL2FFO11FD051 dot protection dot gbl> <537FCEDA dot 9030504 at eagercon dot com> <2e5c185d-329c-46cf-930c-8cc2288891aa at BN1BFFO11FD019 dot protection dot gbl> <538431FB dot 2070904 at eagercon dot com> <865132b2-a593-4147-a7c6-cee25c1ed0fd at BN1AFFO11FD052 dot protection dot gbl> <53909299 dot 9010105 at eagerm dot com>
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