> On Aug. 12, 2012, 9:39 a.m., Ali Saidi wrote: > > This doesn't work. startup is only called before tick 0, so if you restore > > from a checkpoint these events won't got created. by the time loadstate is > > called the system should be all connected up > > > > Nilay Vaish wrote: > Following comment appears in sim_object.hh. As per this, startup() would > be called after > the state has been unserialized. And I did run an Alpha simulation to > check if the function > setupFuncEvents() is getting called or not. In both situations, when > starting bare bones > and when starting from a checkpoint, the function was called. > > /** > * startup() is the final initialization call before simulation. > * All state is initialized (including unserialized state, if any, > * such as the curTick() value), so this is the appropriate place to > * schedule initial event(s) for objects that need them. > */ > virtual void startup(); > > Ali Saidi wrote: > Ok... I stand corrected, but there are still some issues. > > alpha/linux/system.cc still uses initState() for linux specific func > events... as do arm/system, arm/linux/system, sparc, & x86. If there is a > problem with the code in alpha with initState() function all these are going > to have issues too, however fixing them would get rid of the entire point of > loadState and initState, so it seems like load/initState should just make > sure the memory is appropriately setup. Why isn't it? > > > > > Nilay Vaish wrote: > Reading the comments in sim_object.hh and from the code simulate.py, all > initializations, > that depends on some other sim object, should be part of startup(). > initState() and loadState() > are only for individual objects themselves. Since we do not guarantee any > order in which > sim objects are initialized, it is best not to access memory in > initState() / loadState(). > > I can try to fix this for all the architectures. > > Nilay Vaish wrote: > I took a look at other architectures. For no other architecture, > loadState() > is being over ridden. Similarly, initState() is defined only for different > architectures and not for the memory related structures. This means that > memory is ready receive requests from initState() functions. > > I think this patch should be committed as is. No other architecture > requires > any changes, in my opinion. > > > Ali Saidi wrote: > The issue you're fixing in that fixFuncEventAddr() reads the instructions > in memory to understand where to put the event breakpoint, right? > > How is that different from the arm/linux code using the > physProxy.writeBlob() to write the atags structure in memory or the arm > system code writing a branch into the reset vector in initState? Is it > because they'se using the physProxy and alpha is using the virtProxy? I > wouldn't imagine that would matter as both access memory in the same way, one > just does a translation in the process. > > With initState in sparc system loads the reset, openboot, nvram, > hypervison, and partition descriptor into memory. Same question as for arm. > > x86 writes the gdt, page table, smbios, etc into memory via the physproxy > in initState. and the linux specific code writes the e820 and command line. > These are OK? > > I'm fine with these being different is some way, I just don't see how > they are. >
The issue is that the Alpha system is trying to read/write to memory before Ruby has unserialized its state. If this is allowed, then the reads from the memory will potentially return incorrect data and writes to the memory would be over written later when memory state gets unserialized. Note initState() is called only when starting from a cold state, while loadState() is called only when starting from a checkpointed state. Since no memory structures have defined initState() function, this means that all of them are fine with receiving any requests from initState() of any other sim object. That's why Ruby works correctly with Alpha when starting fresh, but fails when starting from a checkpointed state. Does this make things clear? - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1345/#review3260 ----------------------------------------------------------- On Aug. 12, 2012, 9:14 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1345/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2012, 9:14 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9149:56533035907f > --------------------------- > Alpha System: override startup(), instead of loadState() > Alpha System was overriding loadState() function to setup some functional > event. This is not correct since it is not known if the memory has been > correctly setup. This was also happening in the initState() function. With > this patch, Alpha System overrides the startup() function, and sets up > functional events in this function. > > > Diffs > ----- > > src/arch/alpha/system.hh a7a72f42919e > src/arch/alpha/system.cc a7a72f42919e > > Diff: http://reviews.gem5.org/r/1345/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
