From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2945 invoked by alias); 30 Sep 2014 11:43:41 -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 2925 invoked by uid 89); 30 Sep 2014 11:43:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 30 Sep 2014 11:43:39 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8UBhY8h012380 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 30 Sep 2014 07:43:34 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8UBhVwu032120; Tue, 30 Sep 2014 07:43:31 -0400 Message-ID: <542A9762.1060407@redhat.com> Date: Tue, 30 Sep 2014 11:43:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Ajit Kumar Agarwal , Michael Eager , Joel Brobecker CC: "gdb-patches@sourceware.org" , Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: Re: [Patch, microblaze]: Port of Linux gdbserver References: <2570e3c7-f55b-45cd-aa6e-7f4fa145f32a@BN1BFFO11FD002.protection.gbl> <541052B5.5080503@eagercon.com> <20140910134606.GO28404@adacore.com> <050c6461-c35c-441d-9b63-7636d9164e2e@BL2FFO11FD048.protection.gbl> <20140910144313.GP28404@adacore.com> <89d100d8-4ebd-4f50-b5e9-59312124db6a@BL2FFO11FD057.protection.gbl> <54131362.1050009@eagercon.com> <54186D95.4000301@redhat.com> <84be59b9-9f3c-4789-8313-ca3b6061cd1d@BY2FFO11FD003.protection.gbl> <54194300.5090908@redhat.com> <517d120e-6565-406f-acc6-186bb63342fe@BN1BFFO11FD026.protection.gbl> In-Reply-To: <517d120e-6565-406f-acc6-186bb63342fe@BN1BFFO11FD026.protection.gbl> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-09/txt/msg00885.txt.bz2 On 09/23/2014 01:49 PM, Ajit Kumar Agarwal wrote: >>>> >>> Note nothing is done with valid_p. It's write-only. Compare with other ports, like arm-tdep.c or mips-tdep.c. >> > >> > Would look into this and will make the modification. > Thanks. I'd much prefer if we had that patch in the tree before accepting further patches that tweak things around register names, etc. Could you send that (as an independent patch, in a new thread). > +#define microblaze_num_regs \ > + (sizeof microblaze_regmap / sizeof microblaze_regmap[0]) #define microblaze_num_regs ARRAY_SIZE (microblaze_regmap) > + > +static int > +microblaze_cannot_store_register (int regno) > +{ > + if (microblaze_regmap[regno] == -1 || regno == 0) > + return 1; > + > + return 0; > +} > + > +static int > +microblaze_cannot_fetch_register (int regno) > +{ > + return 0; > +} > + > +static CORE_ADDR > +microblaze_get_pc (struct regcache *regcache) > +{ > + unsigned long pc; > + collect_register_by_name (regcache, "rpc", &pc); Empty line after declaration. In several more places in the patch. Please fix them all. > + return (CORE_ADDR) (pc); > +} > + if (regno == 0) > + { > + unsigned long regbuf_0 = 0; > + /* Clobbering r0 so that it is always 0 as enforced by hardware. */ > + supply_register (regcache, regno, (const char*)®buf_0); supply_register_zeroed (regcache, regno); > + } > + else > + { > + if (size < sizeof (long)) > + supply_register (regcache, regno, buf + sizeof (long) - size); > + else > + supply_register (regcache, regno, buf); > + } > +} > + > +/* Provide only a fill function for the general register set. ps_lgetregs > + will use this for NPTL support. */ > + > +static void microblaze_fill_gregset (struct regcache *regcache, void *buf) Line break after "static void". Function name goes on column 0: static void microblaze_fill_gregset (struct regcache *regcache, void *buf) Please make sure that's correct throughout. > +{ > + int i; > + > + for (i = 0; i < microblaze_num_regs; i++) > + microblaze_collect_ptrace_register (regcache, i, > + (char *) buf + microblaze_regmap[i]); > +} > + > +static void > +microblaze_store_gregset (struct regcache *regcache, const void *buf) > +{ > + int i; > + for (i = 0; i < microblaze_num_regs; i++) > + supply_register (regcache, i, (char *) buf + microblaze_regmap[i]); > +} > + > +#endif /* HAVE_PTRACE_GETREGS */ > + > +static struct regset_info microblaze_regsets[] = { > +#ifdef HAVE_PTRACE_GETREGS What's the #ifdef for? Did this kernel port make it upstream without PTRACE_GETREGSET? If there's support for that, can you please switch to using it? PTRACE_GETREGS is supposed to an old way of doing things... > + { PTRACE_GETREGS, PTRACE_SETREGS, 0, 36 * sizeof (elf_gregset_t), > + GENERAL_REGS, microblaze_fill_gregset, microblaze_store_gregset }, > + { 0, 0, 0, -1, -1, NULL, NULL }, > +#endif /* HAVE_PTRACE_GETREGS */ > + { 0, 0, 0, -1, -1, NULL, NULL } > +}; > + > diff --git a/gdb/regformats/microblaze-with-stack-protect.dat b/gdb/regformats/microblaze-with-stack-protect.dat > index f71c111..e349b4a 100644 > --- a/gdb/regformats/microblaze-with-stack-protect.dat > +++ b/gdb/regformats/microblaze-with-stack-protect.dat > @@ -1,7 +1,7 @@ > # DO NOT EDIT: generated from microblaze-with-stack-protect.xml ^^^^^^^^^^^ ^^^^^^^^^^^ ^^^^^^^^^^^ ^^^^^^^^^^^ ^^^^^^^^^^^ Please send a preparatory, independent, patch that updates features/Makefile instead and generates this file, in a new thread, with self-contained description, following the checklist: https://sourceware.org/gdb/wiki/ContributionChecklist > name:microblaze_with_stack_protect > xmltarget:microblaze-with-stack-protect.xml > -expedite:r1,pc > +expedite:r1,rpc > 32:r0 > 32:r1 > 32:r2 > -- 1.7.1 > Thanks, Pedro Alves