From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5753 invoked by alias); 5 Jun 2014 15:54:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 5741 invoked by uid 89); 5 Jun 2014 15:54:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: homiemail-a68.g.dreamhost.com Received: from sub5.mail.dreamhost.com (HELO homiemail-a68.g.dreamhost.com) (208.113.200.129) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Jun 2014 15:54:03 +0000 Received: from homiemail-a68.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a68.g.dreamhost.com (Postfix) with ESMTP id 4C7A040112D31; Thu, 5 Jun 2014 08:54:02 -0700 (PDT) Received: from redwood.eagercon.com (c-24-7-16-38.hsd1.ca.comcast.net [24.7.16.38]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: eager@eagerm.com) by homiemail-a68.g.dreamhost.com (Postfix) with ESMTPSA id 18A7640112D2E; Thu, 5 Jun 2014 08:54:02 -0700 (PDT) Message-ID: <53909299.9010105@eagerm.com> Date: Thu, 05 Jun 2014 15:54:00 -0000 From: Michael Eager User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Ajit Kumar Agarwal , "gdb-patches@sourceware.org" CC: Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: Re: [Patch, microblaze]: Add slr and shr regs References: <537EFA08.1060309@eagercon.com> <537FCEDA.9030504@eagercon.com> <2e5c185d-329c-46cf-930c-8cc2288891aa@BN1BFFO11FD019.protection.gbl> <538431FB.2070904@eagercon.com> <865132b2-a593-4147-a7c6-cee25c1ed0fd@BN1AFFO11FD052.protection.gbl> In-Reply-To: <865132b2-a593-4147-a7c6-cee25c1ed0fd@BN1AFFO11FD052.protection.gbl> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg00270.txt.bz2 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 >> >> * 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