Re: [PATCH] PRU Simulator port

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 2883 bytes --]

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!

try this small form (it has directions in it):
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=doc/Copyright/request-assign.future;hb=HEAD

> > > +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_map,
> > > +		       nbytes, addr, write_transfer,
> > > +		       SIM_SIGSEGV);
> > > +    }
> > > +  else if ((addr >= PC_ADDR_SPACE_MARKER)
> > > +	   || (addr + nbytes > PC_ADDR_SPACE_MARKER))
> > > +    {
> > > +      sim_core_signal (CPU_STATE (cpu), cpu, PC_byteaddr, write_map,
> > > +		       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_map,
> > > +		       nbytes, addr, write_transfer,
> > > +		       SIM_SIGILL);
> > > +    }
> > 
> > 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.
> 
> 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 accessed 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 :(.

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.

>    - 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 ?

>    - Optionally catch NULL pointer dereferences.

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.

> > > +static void
> > > +pru_sim_syscall (SIM_DESC sd, SIM_CPU *cpu)
> > 
> > seems like you should use sim_syscall instead of implementing your own
> > ad-hoc syscall ABI
>
> 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.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]