From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12534 invoked by alias); 18 Jun 2014 14:58:16 -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 12516 invoked by uid 89); 18 Jun 2014 14:58:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: qmta01.emeryville.ca.mail.comcast.net Received: from qmta01.emeryville.ca.mail.comcast.net (HELO qmta01.emeryville.ca.mail.comcast.net) (76.96.30.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Jun 2014 14:58:10 +0000 Received: from omta16.emeryville.ca.mail.comcast.net ([76.96.30.72]) by qmta01.emeryville.ca.mail.comcast.net with comcast id FpuT1o0031ZMdJ4A1qy9mw; Wed, 18 Jun 2014 14:58:09 +0000 Received: from redwood.eagercon.com ([24.7.16.38]) by omta16.emeryville.ca.mail.comcast.net with comcast id Fqy81o00J0pGQcg8cqy8uA; Wed, 18 Jun 2014 14:58:09 +0000 Message-ID: <53A1A900.2000705@eagercon.com> Date: Wed, 18 Jun 2014 14:58: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 , Pedro Alves CC: "gdb-patches@sourceware.org" , Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: Re: [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal. References: <53A023B1.5000105@redhat.com> <859f27cb-8c46-46c1-9625-7287c60f3ae9@BY2FFO11FD007.protection.gbl> In-Reply-To: <859f27cb-8c46-46c1-9625-7287c60f3ae9@BY2FFO11FD007.protection.gbl> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2014-06/txt/msg00666.txt.bz2 On 06/18/14 04:05, Ajit Kumar Agarwal wrote: > Based on the feedback incorporated the changes as mentioned below. Could you please > review the patch and let me know if its okay. From a brief review, the patch looks OK. There were a number of issues raised previously. I haven't reviewed the the comments to see if all have been addressed. I'll let you know if there are any remaining issues. > > > [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal. > > Prior to version MicroBlaze v8.10.a,EDK 13.1, XMD's gdbserver stub returned 57 > registers in response to GDB's G request. Starting with version MicroBlaze > v8.10.a, EDK 13.1, XMD added the slr and shr register, for a count of 59 > registers.This patch adds these registers to the expected G response. This patch > fixes the above problem for baremetal and also supports the backward compatibility. > > ChangeLog: > 2014-06-18 Ajit Agarwal > > * microblaze-tdep.c (microblaze_register_names): Add > the rshr and rslr register names. > (microblaze_gdbarch_init): Use of tdesc_has_registers. > Use of tdesc_find_feature. Use of tdesc_data_alloc. > Use of tdesc_numbered_register. Use of > microblaze_register_g_packet_guesses. Use of > tdesc_use_registers. Use of set_gdbarch_register_type. > (microblaze_register_g_packet_guesses): New. > * 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-core.xml: New file. > * features/microblaze-stack-protect.xml: New file. > * features/microblaze-with-stack-protect.c: New file. > * features/microblaze-with-stack-protect.xml: New file. > * features/Makefile (microblaze-linux): Add > microblaze-linux and microblaze-expedite. > * regformats/microblaze-with-stack-protect.dat: New file. > > Signed-off-by:Ajit Agarwal ajitkum@xilinx.com > > Thanks & Regards > Ajit > > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Tuesday, June 17, 2014 4:47 PM > To: Ajit Kumar Agarwal > Cc: gdb-patches@sourceware.org; Michael Eager; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala > Subject: Re: [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal. > > On 06/12/2014 09:56 AM, Ajit Kumar Agarwal wrote: >> Based on the feedbacks, all changes have been incorporated. >> >> May I know who will committing this patch to the upstream ? >> >> [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal. >> >> Prior to version MicroBlaze v8.10.a,EDK 13.1, XMD's gdbserver stub returned 57 >> registers in response to GDB's G request. Starting with version MicroBlaze >> v8.10.a, EDK 13.1, XMD added the slr and shr register, for a count of 59 >> registers.This patch adds these registers to the expected G response. This >> fixes the above problem for baremetal and also supports the backward compatibility. >> >> ChangeLog: >> >> 2014-06-12 Ajit Agarwal >> >> * microblaze-tdep.c (microblaze_register_names): Add >> the rshr and rslr register names. >> (microblaze_gdbarch_init): Use of tdesc_has_registers. >> Use of tdesc_find_feature. Use of tdesc_data_alloc. >> Use of tdesc_numbered_register. Use of >> microblaze_register_g_packet_guesses. Use of >> tdesc_use_registers. Use of set_gdbarch_register_type. >> (microblaze_register_g_packet_guesses): New. >> >> * 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 >> >> >> 0001-Patch-microblaze-Fix-for-remote-G-Packet-message-too.patch >> >> >> From ce1e6c1f1cd7d49df24727fc179df4c3f6f66bee Mon Sep 17 00:00:00 2001 >> From: Ajit Kumar Agarwal >> Date: Thu, 12 Jun 2014 12:50:16 +0530 >> Subject: [PATCH] [Patch, microblaze]: Fix for remote G Packet message too long error for baremetal. >> >> Prior to version MicroBlaze v8.10.a,EDK 13.1, XMD's gdbserver stub >> returned 57 registers in response to GDB's G request. Starting with >> version MicroBlaze v8.10.a, EDK 13.1, XMD added the slr and shr >> register, for a count of 59 registers.This patch adds these registers >> to the expected G response. This fixes the above problem for baremetal and also supports the backward compatibility. > > Yet, you've named the xml files "linux", and, made the default description assume GNU/Linux osabi. That doesn't look right. > >> >> ChangeLog: >> >> 2014-06-12 Ajit Agarwal >> >> * microblaze-tdep.c (microblaze_register_names): Add >> the rshr and rslr register names. >> (microblaze_gdbarch_init): Use of tdesc_has_registers. >> Use of tdesc_find_feature. Use of tdesc_data_alloc. >> Use of tdesc_numbered_register. Use of >> microblaze_register_g_packet_guesses. Use of >> tdesc_use_registers. Use of set_gdbarch_register_type. >> (microblaze_register_g_packet_guesses): New. >> >> * 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. > > No empty line between entries, please. > >> > >> +++ b/gdb/features/microblaze-cpu.xml >> @@ -0,0 +1,69 @@ >> + >> + >> + >> + > +name="org.gnu.gdb.MicroBlaze.cpu"> >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + > > I don't think this is right. What you want to do, is create a "core" feature, that includes only the set of generic purpose registers that are always available in all microblaze configurations. As discussed, rslr and rshr aren't of that kind. Note that most ports call that feature "core": > > $ grep "org.gnu" * | grep cpu > mips-tdep.c: "org.gnu.gdb.mips.cpu"); > nios2-tdep.c: feature = tdesc_find_feature (tdesc, "org.gnu.gdb.nios2.cpu"); > > $ grep "org.gnu" * | grep core > aarch64-tdep.c: feature = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.core"); > arm-tdep.c: "org.gnu.gdb.arm.core"); > i386-tdep.c: feature_core = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.core"); > m68k-tdep.c: "org.gnu.gdb.m68k.core"); > m68k-tdep.c: "org.gnu.gdb.coldfire.core"); > m68k-tdep.c: "org.gnu.gdb.fido.core"); > rs6000-tdep.c: "org.gnu.gdb.power.core"); > s390-linux-tdep.c: feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.core"); > tic6x-tdep.c: feature = tdesc_find_feature (tdesc, "org.gnu.gdb.tic6x.core"); > > Btw, note how feature names are all lowercase, so > > "org.gnu.gdb.microblaze" > > would be more standard. > > Then, you'll want to create another xml description feature for the C_USE_STACK_PROTECTION microblaze feature, like: > > > > > > > > Finally, a machine that has the stack protection feature enabled can report a description that makes use of xi:include, like: > > > > > > > There are several examples of this in the features/ directory. > E.g., see arm-with-vfpv2.xml. > >> +static void >> +microblaze_register_g_packet_guesses (struct gdbarch *gdbarch) { >> + register_remote_g_packet_guess (gdbarch, >> + MICROBLAZE_NUM_REGS, >> + tdesc_microblaze_linux); >> >> + register_remote_g_packet_guess (gdbarch, >> + MICROBLAZE_NUM_REGS-2, >> + tdesc_microblaze_linux); > > Now here you'll want to guess different descriptions. The second case should guess a description that doesn't include rslr and rshr at all. > > Formatting isn't following the BTW, coding conventions. > Spaces around '-' missing and statements should indented with > 2 spaces, not 6. > > Thanks, > -- > Pedro Alves > -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077