Quoting Joel Hestness <[email protected]>:
Hey Gabe,
Comments are in-lined below. If you'd like me to resubmit another review
of all or part, just let me know.
Thanks,
Joel
util/m5/Makefile.x86
<http://reviews.m5sim.org/r/64/#comment248>
Why is this necessary? Is this so it runs under SE mode? In that case I
think we should make it run like before as the default since 99% of the time
this will run in FS, and provide a way to inject -static for the 1% of the
time it runs in SE.
Compiling it as static all the time wouldn't be the end of the world,
but it seems like we'd be making universal changes for a very uncommon case.
Building the m5 binary without -static allows it to dynamically link a few
libraries:
j...@capillary:~/research/m5-new/util/m5$ ldd m5
linux-vdso.so.1 => (0x00007fff3f9ff000)
libc.so.6 => /lib/libc.so.6 (0x00007fb05131f000)
/lib64/ld-linux-x86-64.so.2 (0x00007fb05168f000)
When I was putting together a disk image using busybox, it had issues with
library versions. In general, since the m5 utility isn't performance
critical and just implements simulator magic, I think it would be easiest if
it was always built statically whether for FS or SE. On the other, I would
imagine that it's built very infrequently and only for initial disk image
creation, so perhaps its not worth changing.
You have a good reason to build it statically, but it would still be
best, I think, if that was optional. I'm not sure what the best way to
do that would be. Maybe another target? Leaving LDFLAGS in the command
line but not setting it to anything might work too, although then
there's the danger of picking something up from the environment. I
don't think it's worth being too elaborate for. Another question might
be why we have three different nearly identical make files instead of
one, or maybe one with really short ones for each arch, or even just
pulling it into scons. Maybe to avoid complicating things too much
this would be its own little scons environment independent from the
main one.
I think there are enough unanswered (but not serious) questions here
that we might want to wait to check this part in.
util/m5/m5ops.h
<http://reviews.m5sim.org/r/64/#comment249>
It looks like Ali commandeered that value on line 61. It might have been
better to use 0x5A for that, but it also might not be safe to change it now
since there may be binaries out there that use it (probably not too many).
It would be a little strange, but you could actually use 0x5A for
reserved1_func. I don't know what restrictions there are in the various ISAs
for function numbers, but in x86 it's a 16 bit value.
Ah, I didn't see that originally!
The only real trouble right now is that if you try to build the m5 utility
for x86_64 with the current version in the repo, it will fail with an
undefined reference to reserved1_func:
gcc -O2 -o m5op_x86.o -c m5op_x86.S
gcc -o m5 m5.o m5op_x86.o
m5op_x86.o: In function `m5_reserved1_func':
(.text+0x5c): undefined reference to `reserved1_func'
collect2: ld returned 1 exit status
make: *** [m5] Error 1
It looks like neither m5op_alpha.S or m5op_sparc.S use reserved1_func, so
another solution would be to remove it from m5op_x86.S (eliminate it
completely from the m5 utility codebase).
Hmm, maybe we should be building these regularly too... What do you
think, Ali? Would it be possible to return reserved1_func and use a
different code?
Gabe
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
