This is the mail archive of the gdb@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Add SystemV IPC drivers to PSIM


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]