----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/#review5914 -----------------------------------------------------------
Took me longer to review this just because I'm not sure what to think of it. It's not pretty, but I don't have better ideas, so it's hard to object. One thing that bothers me is that, while it's noble to try and generalize things, despite the generic-sounding "--external-memory-system" switch, many of the low-level details seem very sst-specific (like 'port_type="sst"' as an obvious one). Would it be more appropriate just to call the option "--sst-memory"? Also, the ExternalCache thing looks like it should be a subclass, even though it's just a function. I can see how the function is more convenient, but making it a subclass would allow some better encapsulation of the cpu_side __getattr__/__setattr__ hack. These are just general thoughts though... I'm very interested in others' input. configs/common/CacheConfig.py <http://reviews.gem5.org/r/2668/#comment5182> This seems obviously specific to sst here configs/common/CacheConfig.py <http://reviews.gem5.org/r/2668/#comment5183> I assume this naming is also specific to sst src/mem/ExternalSlave.py <http://reviews.gem5.org/r/2668/#comment5184> To me, this clearly straddles the line between "clever" and "horrifying" :). If nothing else, it would be nice to put these in an ExternalCache subclass. - Steve Reinhardt On Feb. 20, 2015, 10:47 a.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2668/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2015, 10:47 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10709:a3b771cd744c > --------------------------- > config: Support full-system with SST's memory system > > This patch adds an example configuration in ext/sst/tests/ that allows > an SST/gem5 instance to simulate a 4-core AArch64 system with SST's > memHierarchy components providing all the caches and memories. > > > Diffs > ----- > > configs/common/CacheConfig.py c6cb94a14fea4c59780d73d1623d7031bcede6af > configs/common/FSConfig.py c6cb94a14fea4c59780d73d1623d7031bcede6af > configs/common/MemConfig.py c6cb94a14fea4c59780d73d1623d7031bcede6af > configs/common/Options.py c6cb94a14fea4c59780d73d1623d7031bcede6af > configs/example/fs.py c6cb94a14fea4c59780d73d1623d7031bcede6af > ext/sst/tests/test6_arm_4c.py PRE-CREATION > src/dev/arm/RealView.py c6cb94a14fea4c59780d73d1623d7031bcede6af > src/mem/ExternalSlave.py c6cb94a14fea4c59780d73d1623d7031bcede6af > > Diff: http://reviews.gem5.org/r/2668/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
