Re: Add SystemV IPC drivers to PSIM

[Joel Sherrill <joel.sherrill@oarcorp.com>]

Doug Evans wrote:
> On Fri, Nov 7, 2008 at 12:57 PM, Joel Sherrill
> <joel.sherrill@oarcorp.com> wrote:
>   
>> Daniel Jacobowitz wrote:
>>     
>>> On Fri, Nov 07, 2008 at 12:50:29PM -0600, Joel Sherrill wrote:
>>>
>>>       
>>>> Most of it is boilerplate for adding a device.  Looking
>>>> at the ChangeLog, Andrew hasn't done anything
>>>> except commit someone else's patch in a few years.
>>>> Is anyone other than Andrew capable of reviewing it?
>>>>
>>>>         
>>> You might try asking H-P (Hans-Peter Nilsson <hp@axis.com>).
>>> Beyond that, I'm not sure what to say; no one is really taking care of
>>> these simulators recently.
>>>
>>>
>>>       
>> Thanks. I have emailed him.
>>
>> Hopefully we can turn up someone. If not, what's next?
>> We are using these patches in the RTEMS GDB RPMs and
>> reporting test results to gcc-testresults regularly with these
>> simulators.
>>     
>
> Hi.  Once upon a time I hacked in the sim tree.  What kind of review
> is needed?  I looked over the patch.  It's pretty self-contained so
> the risk is pretty low, and I trust you. :-)
>
> For reference: http://sourceware.org/ml/gdb/2008-09/msg00047.html
>
> The coding style seems to follow the psim style with maybe one caveat.
>  There's very little pretty-alignment like this in psim:
>
>
>   
Got all the indentation I think.
> I can't tell from reading the code what will break if sizeof(int) !=
> 4.  How about a comment?
> [lots may break, but why check for it here is what I'm getting at]
> Plus could "typing problem" be more specific?
>
> +  if (sizeof(int) != 4)
> +    error("hw_sem_init_data() typing problem\n");
> l
> I don't have an answer for this, but I don't see any other hw*.c
> verifying the address either:
>   
You are right.  This is unclear and now unnecessary.  The problem
was that the "registers" on the device must match the
size of the integer returned by the native semctl()
call for semaphore count.  All accesses must be
4 bytes wide in the semaphore access code.

But the code that had to deal with this has been updated
to explicitly move the semaphore count into an unsigned32
so there is no restriction now.

So I removed this.
> +  /* do we need to worry about out of range addresses? */
>
> I think there's a cut-n-paste error in the sem docs.  Should the
> second 0xc0000000 be 0xfff00000?
> I don't completely understand the device configuration syntax, but the
> code checks for "value" yet you're using "reg" in the docs.  Is that a
> problem? [perhaps just more of the same cut-n-paste error of course]
> Maybe something like -o '/sim@0xfff00000/value 0' or some such instead
> of -o '/sem@0xfff00000/reg 0xfff00000 0x80000'  ?
>
>   
The syntax is hard to grok.  I fixed this to match what
I use in my script that runs psim.  0xc0000000 for 12 bytes.
> +   Configure a UNIX semaphore using key 0x12345678 mapped into psim
> +   address space at 0xfff00000:
> +
> +   |  -o '/sem@0xfff00000/reg 0xfff00000 0x80000' \
> +   |  -o '/sem@0xfff00000/key 0x12345678' \
> +
> +   sim/ppc/run -o '/#address-cells 1' \
> +         -o '/sem@0xc0000000/reg 0xc0000000 0x80000' \
> +         -o '/sem@0xc0000000/key 0x12345678' ../psim-hello/hello
>
> Other than these nits, it's ok with me (fwiw of course).
>   
Thanks.  I think I have gotten these fixed.

I will test a bit and resubmit a patch.

Thanks.

--joel