From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9283 invoked by alias); 9 Jul 2013 18:23:52 -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 9273 invoked by uid 89); 9 Jul 2013 18:23:51 -0000 X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_YE,SPF_PASS autolearn=ham version=3.3.1 Received: from mail-pd0-f173.google.com (HELO mail-pd0-f173.google.com) (209.85.192.173) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 09 Jul 2013 18:23:50 +0000 Received: by mail-pd0-f173.google.com with SMTP id v14so5476549pde.18 for ; Tue, 09 Jul 2013 11:23:49 -0700 (PDT) X-Received: by 10.68.217.137 with SMTP id oy9mr27643655pbc.130.1373394229298; Tue, 09 Jul 2013 11:23:49 -0700 (PDT) Received: from [192.168.1.5] (114-25-181-13.dynamic.hinet.net. [114.25.181.13]) by mx.google.com with ESMTPSA id iq6sm29532679pbc.1.2013.07.09.11.23.45 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 09 Jul 2013 11:23:48 -0700 (PDT) Message-ID: <51DC552D.3060900@gmail.com> Date: Tue, 09 Jul 2013 18:23:00 -0000 From: Wei-cheng Wang User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 1/5] Code for nds32 target References: <51DB8D14.20205@codesourcery.com> In-Reply-To: <51DB8D14.20205@codesourcery.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-07/txt/msg00251.txt.bz2 Hi Yao Qi, Thanks for you reviewing and suggestion. I will revise the patch based on your suggestion thoroughly :) Here are some brief answers and questions. On 7/9/2013 12:09 PM, Yao Qi wrote: > On 07/08/2013 05:27 PM, Wei-cheng Wang wrote: > Do we really need "regnum="15"" here? Yes, but it is unnecessary, so I will remove this. It was here because r11-r14 does not exist in reduced register configuration. > Since there are no comments on what nds32-remote.c is for, I assume that > it is used to talk with remote target, like jtag ice. IIUC, you invent > some new rsp packets to your own targets, but it is not recommended to > add such thing into GDB nowadays. I can't find the mail archive about > this decision, and other people can give a definitive answer here. Yes, this file are used to talk with our remote OpenOCD and SID. If you mean those "qPart:nds32:*" packets, I agree we should remove them and our remote target should be changed accordingly for conforming GDB RSP standard. Or do you mean monitor commands ("qRcmd,command" packetss) are also not recommended in GDB nowadays? >> +} >> + >> +/* Implement the gdbarch_breakpoint_from_pc method. */ >> + >> +static const gdb_byte * >> +nds32_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, >> + int *lenptr) >> +{ >> + static const gdb_byte NDS32_BREAK16[] = { 0xEA, 0x00 }; > > Do we need to worry about big-endian and little endian here? Instructions are always big-endian. >> + if ((*pcptr) & 1) >> + error (_("Bad address %p for inserting breakpoint.\n" >> + "Address must be at least 2-byte aligned."), (void *) *pcptr); > > Do you know when the last bit of address is 1? I am wondering whether > this checking is necessary or it is caused by the bug somewhere else. Just in case users may set the breakpoint in address directly. (gdb) break *0x4321 I think I would better stop them at the first place. > Can you define 'fucpr' in the target description? like > > > > ..... > > > which is simpler, IMO. > >> + >> + Return the GDB type object for the "standard" data type >> + of data in register N. >> + It get pretty messy here. I need enum-types and bit-fields >> + for better representation. But they cannot be done by >> + tdesc-xml. */ > > target description works for enum and bitfields. For example, in > features/i386/32bit-core.xml, > Target description can not describe a enum-typed bit-field. If bit-field struct is used, I could not specify the type for a bit-field. ... If I want to specify field type, I couldn't specify size of a field. ... All I want is to display a register in such format. (gdb) p $cr0 $1 = {CFGID = [ PERF_EXT 16_EXT PERF_EXT2 COP_EXT STR_EXT ], REV = 16, CPUID = N13} (cr0 consist of config flags, revision number and CPU ID.) I agree all these type describing thing should be in target-description, but if target-description could not describe all the needed types, IMHO managing them in a single place may be better then scattering them in GDB and target-description. Or I may help to extend target-description, so we could specify types of bit-fields? Any suggestion? > I have to stop reviewing here as I've run out of time of patch review > today. I may have a look at the rest of the patch tomorrow. Thanks for your great help :) Wei-cheng