From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23684 invoked by alias); 24 Jun 2014 10:28:21 -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 23674 invoked by uid 89); 24 Jun 2014 10:28:20 -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_LOW,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: na01-bl2-obe.outbound.protection.outlook.com Received: from mail-bl2lp0204.outbound.protection.outlook.com (HELO na01-bl2-obe.outbound.protection.outlook.com) (207.46.163.204) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 24 Jun 2014 10:28:16 +0000 Received: from BY2FFO11FD038.protection.gbl (10.1.14.30) by BY2FFO11HUB057.protection.gbl (10.1.15.232) with Microsoft SMTP Server (TLS) id 15.0.969.12; Tue, 24 Jun 2014 10:28:05 +0000 Received: from xsj-pvapsmtpgw01 (149.199.60.83) by BY2FFO11FD038.mail.protection.outlook.com (10.1.14.223) with Microsoft SMTP Server (TLS) id 15.0.969.12 via Frontend Transport; Tue, 24 Jun 2014 10:28:05 +0000 Received: from unknown-38-66.xilinx.com ([149.199.38.66] helo=xsj-smtp1) by xsj-pvapsmtpgw01 with esmtp (Exim 4.63) (envelope-from ) id 1WzNxP-0000Lc-Vr; Tue, 24 Jun 2014 03:28:07 -0700 From: Ajit Kumar Agarwal To: Pedro Alves 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. Date: Tue, 24 Jun 2014 10:28:00 -0000 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> In-Reply-To: <53A94147.4050700@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-RCIS-Action: ALLOW Message-ID: <57ebe4b0-83eb-4208-9778-472ecf0048d4@BY2FFO11FD038.protection.gbl> X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:149.199.60.83;CTRY:US;IPV:NLI;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(6009001)(438002)(51704005)(479174003)(13464003)(377454003)(189002)(199002)(76482001)(46406003)(86362001)(74316001)(19580395003)(83322001)(83072002)(85852003)(81342001)(70736001)(106116001)(53416004)(46102001)(47776003)(99396002)(64706001)(50466002)(6806004)(85306003)(31696002)(33646001)(54356999)(74502001)(31966008)(50986999)(106466001)(4396001)(19580405001)(74662001)(76176999)(2656002)(44976005)(20776003)(1496007)(77982001)(87936001)(80022001)(81542001)(92566001)(104016002)(79102001)(77096002)(92726001)(21056001)(97756001)(93886003)(95666004)(23726002)(23106004);DIR:OUT;SFP:;SCL:1;SRVR:BY2FFO11HUB057;H:xsj-pvapsmtpgw01;FPR:;MLV:sfv;PTR:unknown-60-83.xilinx.com;MX:1;A:1;LANG:en; X-OriginatorOrg: xilinx.onmicrosoft.com X-Microsoft-Antispam: BCL:0;PCL:0;RULEID: X-Forefront-PRVS: 02524402D6 Received-SPF: Pass (: domain of xilinx.com designates 149.199.60.83 as permitted sender) receiver=; client-ip=149.199.60.83; helo=xsj-pvapsmtpgw01; Authentication-Results: spf=pass (sender IP is 149.199.60.83) smtp.mailfrom=ajit.kumar.agarwal@xilinx.com; X-SW-Source: 2014-06/txt/msg00840.txt.bz2 -----Original Message----- From: Pedro Alves [mailto:palves@redhat.com]=20 Sent: Tuesday, June 24, 2014 2:44 PM To: Ajit Kumar Agarwal Cc: gdb-patches@sourceware.org; Michael Eager; Vinod Kathail; Vidhumouli Hu= nsigida; Nagaraju Mekala Subject: Re: [Patch, microblaze]: Fix for remote G Packet message too long = error for baremetal. Hi, >>I read this again and found things I should have mentioned before or thin= gs I mentioned before but weren't addressed. See below. Thanks !! On 06/23/2014 08:36 PM, Ajit Kumar Agarwal wrote: > diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c index=20 > 14c1b52..c57437c 100644 > --- a/gdb/microblaze-tdep.c > +++ b/gdb/microblaze-tdep.c > @@ -33,13 +33,15 @@ > #include "frame-unwind.h" > #include "dwarf2-frame.h" > #include "osabi.h" > - > +#include "features/microblaze-with-stack-protect.c" > +#include "features/microblaze.c" > #include "gdb_assert.h" >>A little odd to see the .c files included in the middle of the .h files. = All other files seem to include the ".c" files after all the .h files. I = wonder which existing file you modelled from? I will incorporate this change. > #include > #include "target-descriptions.h" > #include "opcodes/microblaze-opcm.h" > #include "opcodes/microblaze-dis.h" > #include "microblaze-tdep.h" > +#include "remote.h" >=20=20 >=20 > /* Instruction macros used for analyzing the prologue. */ > /* This set of instruction macros need to be changed whenever the @@=20 > -73,7 +75,8 @@ static const char *microblaze_register_names[] =3D > "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",=20=20 > + "rslr", "rshr" > }; >=20=20 > #define MICROBLAZE_NUM_REGS ARRAY_SIZE (microblaze_register_names) @@=20 > -663,17 +666,66 @@ microblaze_dwarf2_reg_to_regnum (struct gdbarch *gdbar= ch, int reg) > gdb_assert (reg < sizeof (dwarf2_to_reg_map)); > return dwarf2_to_reg_map[reg]; > } > +static void >>Please make you sure there's an empty line between functions. I believe = I commented on this before. I will make this change. > +microblaze_register_g_packet_guesses (struct gdbarch *gdbarch) { > + register_remote_g_packet_guess (gdbarch, > + MICROBLAZE_NUM_CORE_REGS,=20 > + tdesc_microblaze); >=20=20 > + register_remote_g_packet_guess (gdbarch, > + MICROBLAZE_NUM_REGS, > +=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20 > +tdesc_microblaze_with_stack_protect); > +} > static struct gdbarch * Here too. > microblaze_gdbarch_init (struct gdbarch_info info, struct=20 > gdbarch_list *arches) { > struct gdbarch_tdep *tdep; > struct gdbarch *gdbarch; > + struct tdesc_arch_data *tdesc_data =3D NULL; const struct=20 > + target_desc *tdesc =3D info.target_desc; >=20=20 > /* If there is already a candidate, use it. */ > arches =3D gdbarch_list_lookup_by_info (arches, &info); > if (arches !=3D NULL) > return arches->gdbarch; > + if (tdesc =3D=3D NULL) > + tdesc =3D tdesc_microblaze_with_stack_protect; Shouldn't the default be to _not_ assume stack protect ? The default is choosen to assume stack protect to make compatible with the = handling of stack protect registers in XMD Debugger. > + /* Check any target description for validity. */ if=20 > + (tdesc_has_registers (tdesc)) > + { > + const struct tdesc_feature *feature; > + int valid_p; > + int i; > + > + feature =3D tdesc_find_feature (tdesc, > + "org.gnu.gdb.microblaze.core"); > + if (feature =3D=3D NULL) > + return NULL; > + tdesc_data =3D tdesc_data_alloc (); > + > + valid_p =3D 1; > + for (i =3D 0; i < MICROBLAZE_NUM_CORE_REGS; i++) > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, i, > + microblaze_register_names[i]= ); > + feature =3D tdesc_find_feature (tdesc, > +=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20 > + "org.gnu.gdb.microblaze.stack-protect"); > + > + if (feature !=3D NULL) > + { > + valid_p =3D 1; > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, > + MICROBLAZE_SLR_REGNUM, > + "rslr"); > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, > + MICROBLAZE_SHR_REGNUM, > + "rshr"); > + } > + } > + tdep =3D xcalloc (1, sizeof (struct gdbarch_tdep)); gdbarch =3D=20 > + gdbarch_alloc (&info, tdep); > + > + microblaze_register_g_packet_guesses (gdbarch); >=20=20 > /* Allocate space for the new architecture. */ > tdep =3D XNEW (struct gdbarch_tdep); > @@ -725,7 +777,11 @@ microblaze_gdbarch_init (struct gdbarch_info info, s= truct gdbarch_list *arches) > dwarf2_append_unwinders (gdbarch); > frame_unwind_append_unwinder (gdbarch, µblaze_frame_unwind); > frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer); > - > + if (tdesc_data !=3D 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 fo= r 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 t= he microblaze_register_type always return builtin_int for these stack prote= ct registers. > + } > return gdbarch; > } >=20=20 > + /* Offsets to saved registers. */ > + int register_offsets[MICROBLAZE_NUM_REGS]; /* Must match MICROBLAZE= _NUM_REGS. */ >>The "Must match MICROBLAZE_NUM_REGS" comment now looks unnecessary to me. I will make this change. >>As I mentioned before, please don't forget to document the new target fea= tures in the manual. Would you mind in explaining which manual need to be changed for the new ta= rget. -- Pedro Alves