For what it does, this code seems fine, but before it gets committed I'd like to have a little more open-ended discussion about what the Platform object is good for... why is it even separate from System? Is there ever anything other than a 1:1 relationship in FS mode, and if not, are there other benefits to having Platform be separate?
As I mentioned in another review, I've been playing around recently with compiling a device into SE mode, and this Platform thing turned out to be a pain point. i ended up working around it in a different way from Gabe, but I'd like to know whether we should just get rid of it instead of finding ways to work around it. If there are aspects of a System object that only make sense in FS mode, then let's just make them optional parts of System, or create a FSSystem derived class that has them, or something like that. The bottom line is that I think Platform is mostly historical, but I don't know the history, so maybe for starters someone like Nate or Ali could bring me up to speed... Steve On Mon, Sep 26, 2011 at 2:21 AM, Gabe Black <[email protected]> wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/887/ > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > By Gabe Black. > Description > > SE/FS: Remove System::platform and Platform::intrFrequency. > > In order for a system object to work in SE mode and FS mode, it has to either > always require a platform object even in SE mode, or get rid of the > requirement all together. Making SE mode carry around unnecessary/unused bits > of FS seems less than ideal, so I decided to go with the second option. The > platform pointer in the System class was used for exactly one purpose, a path > for the Alpha Linux system object to get to the real time clock and read its > frequency so that it could short cut the loops_per_jiffy calculation. There > was also a copy and pasted implementation in MIPS, but since it was only there > because it was there in Alpha I still count that as one use. > > This change reverses the mechanism that communicates the RTC frequency so that > the Tsunami platform object pushes it up to the AlphaSystem object. This is > slightly less specific than it could be because really only the > AlphaLinuxSystem uses it. Because the intrFrequency function on the Platform > class was no longer necessary (and unimplemented on anything but Alpha) it was > eliminated. > > After this change, a platform will need to have a system, but a system won't > have to have a platform. > > Diffs > > - src/arch/alpha/linux/system.cc (cf26f9578cd0) > - src/arch/alpha/system.hh (cf26f9578cd0) > - src/arch/alpha/system.cc (cf26f9578cd0) > - src/arch/mips/linux/system.cc (cf26f9578cd0) > - src/dev/alpha/backdoor.cc (cf26f9578cd0) > - src/dev/alpha/tsunami.hh (cf26f9578cd0) > - src/dev/alpha/tsunami.cc (cf26f9578cd0) > - src/dev/arm/realview.hh (cf26f9578cd0) > - src/dev/arm/realview.cc (cf26f9578cd0) > - src/dev/mips/malta.hh (cf26f9578cd0) > - src/dev/mips/malta.cc (cf26f9578cd0) > - src/dev/platform.hh (cf26f9578cd0) > - src/dev/sparc/t1000.hh (cf26f9578cd0) > - src/dev/sparc/t1000.cc (cf26f9578cd0) > - src/dev/x86/pc.hh (cf26f9578cd0) > - src/dev/x86/pc.cc (cf26f9578cd0) > - src/sim/system.hh (cf26f9578cd0) > > View Diff <http://reviews.m5sim.org/r/887/diff/> > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
