From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66392 invoked by alias); 18 Dec 2016 20:13:49 -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 66382 invoked by uid 89); 18 Dec 2016 20:13:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=2.0 required=5.0 tests=AWL,BAYES_00,BODY_8BITS,GARBLED_BODY autolearn=no version=3.3.2 spammy==d0=bf=d0=b5=d1, region, enforce, Simulator?= X-HELO: server28.host.bg Received: from server28.host.bg (HELO server28.host.bg) (87.120.40.98) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 18 Dec 2016 20:13:38 +0000 Received: from [95.87.234.74] (port=59828 helo=tpdeb.localnet) by server28.host.bg with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87) (envelope-from ) id 1cIhpu-002ThA-2s; Sun, 18 Dec 2016 22:13:34 +0200 From: Dimitar Dimitrov To: Mike Frysinger Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] PRU Simulator port Date: Sun, 18 Dec 2016 20:13:00 -0000 Message-ID: <3424509.8t91nzmn7c@tpdeb> User-Agent: KMail/5.2.3 (Linux/4.8.0-1-amd64; KDE/5.27.0; x86_64; ; ) In-Reply-To: <20161216203324.GA30602@vapier.lan> References: <20161205211108.26616-1-dimitar@dinux.eu> <9877120.H2PyqMF4WO@tpdeb> <20161216203324.GA30602@vapier.lan> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" X-Get-Message-Sender-Via: server28.host.bg: authenticated_id: dimitar@dinux.eu X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00335.txt.bz2 On =D0=BF=D0=B5=D1=82=D1=8A=D0=BA, 16 =D0=B4=D0=B5=D0=BA=D0=B5=D0=BC=D0=B2= =D1=80=D0=B8 2016 =D0=B3. 15:33:24 EET Mike Frysinger wrote: > On 09 Dec 2016 22:39, Dimitar Dimitrov wrote: > > I'll change it. I assume my copyright papers for GDB will cover this > > simulator patch? > i think they want sep papers. who knows! >=20 > try this small form (it has directions in it): > http://git.savannah.gnu.org/gitweb/?p=3Dgnulib.git;a=3Dblob_plain;f=3Ddoc= /Copyrigh > t/request-assign.future;hb=3DHEAD Thanks, I've started the process. > > > > +static inline void > > > > +pru_reg2dmem (SIM_CPU *cpu, uint32_t addr, unsigned int nbytes, > > > > + int regn, int regb) > > > > +{ > > > > + if (abort_on_dmem_zero_access && addr < 4) > > > > + { > > > > + sim_core_signal (CPU_STATE (cpu), cpu, PC_byteaddr, write_ma= p, > > > > + nbytes, addr, write_transfer, > > > > + SIM_SIGSEGV); > > > > + } > > > > + else if ((addr >=3D PC_ADDR_SPACE_MARKER) > > > > + || (addr + nbytes > PC_ADDR_SPACE_MARKER)) > > > > + { > > > > + sim_core_signal (CPU_STATE (cpu), cpu, PC_byteaddr, write_ma= p, > > > > + nbytes, addr, write_transfer, > > > > + SIM_SIGSEGV); > > > > + } > > > > + else if ((regn * 4 + regb + nbytes) > (32 * 4)) > > > > + { > > > > + /* Register and load size are not valid. */ > > > > + sim_core_signal (CPU_STATE (cpu), cpu, PC_byteaddr, write_ma= p, > > > > + nbytes, addr, write_transfer, > > > > + SIM_SIGILL); > > > > + } > > >=20 > > > do you really need to do all this ? seems like the existing > > > sim_core_write_1 function already deals properly with writes > > > to out-of-bind addresses. > >=20 > > I believe the checks are needed. I let sim_core_write_1 handle the Data= RAM bounds checking. On top of that, I want to: > > - Ensure that instruction memory (PC_ADDR_SPACE_MARKER) is not acces= sed > > by the CPU data path. > i don't believe this is possible today with gdb. when gdb makes a request > to the sim to do things like set breakpoints and decode instructinos, it > follows the same paths as the sim which means it'll break gdb :(. >=20 > i wanted to do the same thing for Blackfin where there's a chunk of SRAM > that can only be executed, but trying to do data reads/stores would fail. May I conditionally enforce this restriction only if "(STATE_OPEN_KIND (sd)= =3D=3D SIM_OPEN_STANDALONE)" ? The standalone simulator is invaluable for = regression checking Binutils and GCC, and I would like to catch those kinds= of errors. >=20 > > - Check that the load/store burst length is valid (i.e. do not access > > beyond the last CPU register). > shouldn't this be handled by limiting the memory region addr+size ? PRU LBBO instruction can load up to 120 bytes into a group of consecutive r= egisters. This check guards the destination register group, not the source = memory address. Consider an example, where r10 holds a valid address in the middle of a lar= ge array. This would be a valid instruction for loading 32 bytes (8 words) = into r20-r27: ldi &r20, r10, 0, 32 And this would be invalid because it would try to load 8 words into r29-r36= , while PRU registers are only r0-r31: ldi &r29, r10, 0, 32 >=20 > > - Optionally catch NULL pointer dereferences. >=20 > hmm, seems like this would be better as a common sim option ? but also, > this runs into the same problems as i described above i think. Again, may I enable it only for standalone simulator? >=20 > > > > +static void > > > > +pru_sim_syscall (SIM_DESC sd, SIM_CPU *cpu) > > >=20 > > > seems like you should use sim_syscall instead of implementing your own > > > ad-hoc syscall ABI > >=20 > > I'll fix libgloss to use more standard syscalls, and then I'll switch to > > sim_syscall. > that was going to be my next question :). if you have time, then yeah, > changing all your libgloss syscall #'s to use the default ones would be > best. I will do that. > -mike