From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23951 invoked by alias); 30 Jun 2014 14:47:39 -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 23938 invoked by uid 89); 30 Jun 2014 14:47:37 -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: qmta14.emeryville.ca.mail.comcast.net Received: from qmta14.emeryville.ca.mail.comcast.net (HELO qmta14.emeryville.ca.mail.comcast.net) (76.96.27.212) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Jun 2014 14:47:35 +0000 Received: from omta03.emeryville.ca.mail.comcast.net ([76.96.30.27]) by qmta14.emeryville.ca.mail.comcast.net with comcast id Le3Z1o0050b6N64AEena6L; Mon, 30 Jun 2014 14:47:34 +0000 Received: from redwood.eagercon.com ([24.7.16.38]) by omta03.emeryville.ca.mail.comcast.net with comcast id LenY1o00X0pGQcg8PenZdK; Mon, 30 Jun 2014 14:47:33 +0000 Message-ID: <53B17884.1060202@eagercon.com> Date: Mon, 30 Jun 2014 14:47: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> <53A1ABF0.9080004@redhat.com> <74281fd5-518a-4d7f-977a-6fa1320f6db9@BY2FFO11FD016.protection.gbl> <53A1B61F.9080803@redhat.com> <736c2e0d-6ff1-40c3-8120-dc6f5d91e6b1@BL2FFO11FD052.protection.gbl> <53A8290A.1050701@redhat.com> <53A94147.4050700@redhat.com> <57ebe4b0-83eb-4208-9778-472ecf0048d4@BY2FFO11FD038.protection.gbl> <53A96993.5040804@redhat.com> <109c35c1-e2f6-430f-9235-c6c82a93daf1@BL2FFO11FD009.protection.gbl> <53A97330.4080708@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2014-06/txt/msg00959.txt.bz2 On 06/30/14 03:32, Ajit Kumar Agarwal wrote: > Based on the feedback, updated the patch with the review comments. How has this been tested? Please be specific. > > [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-26 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 MICROBLAZE_SHR_REGNUM > MICROBLAZE_NUM_REGS and MICROBLAZE_NUM_CORE_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/microblaze.xml: New file. > * features/microblaze.c: New file. > * features/Makefile (microblaze-with-stack-protect): Add > microblaze-with-stack-protect microblaze and > microblaze-expedite. > * regformats/microblaze-with-stack-protect.dat: New file. > * regformats/microblaze.dat: New file. > * doc/gdb.texinfo (MicroBlaze Features): New. > > Signed-off-by:Ajit Agarwal ajitkum@xilinx.com > >> In this case is it correct to say >> If (tdesc == NULL) >> tdesc = tdesc_microblaze; >> >> instead of tdesc_microblaze_with_stack_protect? > >>> Yes. > > Instead of tdesc_microblaze_with_stack_protect if I use tdesc_microblaze the "G Packet message is too long" error is not resolved. > The patch is unchanged with tdesc_microblaze_stack_protect. > > Thanks & Regards > Ajit > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Tuesday, June 24, 2014 6:17 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/24/2014 01:31 PM, Ajit Kumar Agarwal wrote: > >>> >>> The default is choosen to assume stack protect to make compatible with the handling of stack protect registers in XMD Debugger. >> >>>> But you've already added the G packet size guess for that. >> >> In this case is it correct to say >> If (tdesc == NULL) >> tdesc = tdesc_microblaze; >> >> instead of tdesc_microblaze_with_stack_protect? > > Yes. > >> >>>> - >>>> + if (tdesc_data != NULL) >>>> + { >>>> + tdesc_use_registers (gdbarch, tdesc, tdesc_data); >>>> + set_gdbarch_register_type (gdbarch, >>>> + microblaze_register_type); >>> >>>>> Hmm, why is this set_gdbarch_register_type call necessary? >>> >>> /* Override tdesc_register_type to adjust the types of VFP >>> registers for NEON. */ >>> This is done for arm target to set the different type for VFP >>> registers for Neon with Boolean flags is set before this call for VFP >>> registers. In the microblaze target it's not required for special >>> case of stack protect as >> the microblaze_register_type always return builtin_int for these stack protect registers. >> >> Right. > > Actually, not right... This comment doesn't really appear to be correct: > >> In the microblaze target it's not required for special case of stack >> protect as the microblaze_register_type always return builtin_int for these stack protect registers. > > > static struct type * > microblaze_register_type (struct gdbarch *gdbarch, int regnum) { > if (regnum == MICROBLAZE_SP_REGNUM) > return builtin_type (gdbarch)->builtin_data_ptr; > > if (regnum == MICROBLAZE_PC_REGNUM) > return builtin_type (gdbarch)->builtin_func_ptr; > > return builtin_type (gdbarch)->builtin_int; } > > MICROBLAZE_SP_REGNUM and MICROBLAZE_PC_REGNUM clearly aren't builtin_int... > > Doesn't your patch change the output of "ptype $sp" and "ptype $pc" ? > > That points at something missing in the target description: > >> + > +name="org.gnu.gdb.microblaze.core"> >> + > ... >> + > > AFAICS, SP is "r1", and PC is "rpc". These should be marked with type="data_ptr" and type="code_ptr" . > >> >>>>> As I mentioned before, please don't forget to document the new target features in the manual. >>> >>> Would you mind in explaining which manual need to be changed for the new target. >> >>>> The GDB manual, gdb/doc/gdb.texinfo, describes all the standard XML target features. See the "Standard Target Features" node, and add a new subsection for MicroBlaze. >> >> Thanks !! I will add subsection for Microblaze target. > > Thank you. > > -- > Pedro Alves > -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077