Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/#review5980 --- Looks much nicer. Thanks for the changes! Sorry for the long wait for the review. Nothing big, just could use a couple more comments I think. configs/common/CacheConfig.py http://reviews.gem5.org/r/2668/#comment5218 a comment explaining the need/purpose for this class would be nice configs/common/CacheConfig.py http://reviews.gem5.org/r/2668/#comment5219 please add a comment here explaining that there is an assumption being made on how the external memory system names its ports ext/sst/tests/test6_arm_4c.py http://reviews.gem5.org/r/2668/#comment5217 so does this require you to edit the file to replace /XXX/abs-path-to? couldn't this path be read from an environment variable, or perhaps you could leave it as a relative path so it would work if you ran from the gem5 directory? - Steve Reinhardt On March 3, 2015, 5:07 p.m., Curtis Dunham wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 3, 2015, 5:07 p.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 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/FSConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/MemConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/Options.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/example/fs.py 8a20e2a1562debfc20b92be4581457c4147b3733 ext/sst/tests/test6_arm_4c.py PRE-CREATION src/dev/arm/RealView.py 8a20e2a1562debfc20b92be4581457c4147b3733 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 27, 2015, 10:27 p.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 (updated) - src/dev/arm/RealView.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/common/MemConfig.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/common/Options.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/example/fs.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e ext/sst/tests/test6_arm_4c.py PRE-CREATION configs/common/CacheConfig.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/common/FSConfig.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
On March 27, 2015, 6:28 p.m., Steve Reinhardt wrote: ext/sst/tests/test6_arm_4c.py, line 93 http://reviews.gem5.org/r/2668/diff/5/?file=43877#file43877line93 so does this require you to edit the file to replace /XXX/abs-path-to? couldn't this path be read from an environment variable, or perhaps you could leave it as a relative path so it would work if you ran from the gem5 directory? Yes, it does. It can work as relative; I will change it to that. - Curtis --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/#review5980 --- On March 27, 2015, 10:27 p.m., Curtis Dunham wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 27, 2015, 10:27 p.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 - src/dev/arm/RealView.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/common/MemConfig.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/common/Options.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/example/fs.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e ext/sst/tests/test6_arm_4c.py PRE-CREATION configs/common/CacheConfig.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e configs/common/FSConfig.py 8a7285d6197eb0d8f1642e6cdb7a21aa1ff6310e 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 4, 2015, 12:56 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 (updated) - configs/common/CacheConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/FSConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/MemConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/Options.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/example/fs.py 8a20e2a1562debfc20b92be4581457c4147b3733 ext/sst/tests/test6_arm_4c.py PRE-CREATION src/dev/arm/RealView.py 8a20e2a1562debfc20b92be4581457c4147b3733 src/mem/ExternalSlave.py 8a20e2a1562debfc20b92be4581457c4147b3733 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 4, 2015, 1:04 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 (updated) - configs/common/CacheConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/FSConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/MemConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/Options.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/example/fs.py 8a20e2a1562debfc20b92be4581457c4147b3733 ext/sst/tests/test6_arm_4c.py PRE-CREATION src/dev/arm/RealView.py 8a20e2a1562debfc20b92be4581457c4147b3733 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 4, 2015, 12: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 (updated) - configs/common/CacheConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/FSConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/MemConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/Options.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/example/fs.py 8a20e2a1562debfc20b92be4581457c4147b3733 ext/sst/tests/test6_arm_4c.py PRE-CREATION src/dev/arm/RealView.py 8a20e2a1562debfc20b92be4581457c4147b3733 src/mem/ExternalSlave.py 8a20e2a1562debfc20b92be4581457c4147b3733 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 4, 2015, 1:07 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 (updated) - configs/common/CacheConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/FSConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/MemConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/Options.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/example/fs.py 8a20e2a1562debfc20b92be4581457c4147b3733 ext/sst/tests/test6_arm_4c.py PRE-CREATION src/dev/arm/RealView.py 8a20e2a1562debfc20b92be4581457c4147b3733 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
On Feb. 25, 2015, 4:32 p.m., Steve Reinhardt wrote: 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. Curtis Dunham wrote: There is some truth to this complaint. I think there is a little room for improvement, though. It's ready for another look at diff r5. - Curtis --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/#review5914 --- On March 4, 2015, 1:07 a.m., Curtis Dunham wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated March 4, 2015, 1:07 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 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/FSConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/MemConfig.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/common/Options.py 8a20e2a1562debfc20b92be4581457c4147b3733 configs/example/fs.py 8a20e2a1562debfc20b92be4581457c4147b3733 ext/sst/tests/test6_arm_4c.py PRE-CREATION src/dev/arm/RealView.py 8a20e2a1562debfc20b92be4581457c4147b3733 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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
On Feb. 25, 2015, 4:32 p.m., Steve Reinhardt wrote: 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. There is some truth to this complaint. I think there is a little room for improvement, though. On Feb. 25, 2015, 4:32 p.m., Steve Reinhardt wrote: configs/common/CacheConfig.py, line 130 http://reviews.gem5.org/r/2668/diff/1/?file=43775#file43775line130 I assume this naming is also specific to sst Not here. The naming just has to agree with the configuration on the other side, which in this case is the ext/sst/tests/test6_arm_4c.py in this patchset. However as long as any other simulator we hook up with is cool with the same naming, it doesn't need to be changed. On Feb. 25, 2015, 4:32 p.m., Steve Reinhardt wrote: configs/common/CacheConfig.py, line 50 http://reviews.gem5.org/r/2668/diff/1/?file=43775#file43775line50 This seems obviously specific to sst here Correct that port_type=sst is SST specific because the ext/sst/Ext{Master,Slave} code registers a handler for handler_name sst. Perhaps --external-memory-system could take a string argument that could be stuck here? We'd use --external-memory-system=sst on the command line. On Feb. 25, 2015, 4:32 p.m., Steve Reinhardt wrote: src/mem/ExternalSlave.py, line 57 http://reviews.gem5.org/r/2668/diff/1/?file=43782#file43782line57 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. Clever/Horrifying... Yes. :) My concern here is that creating a new class doesn't seem to solve the problem. As best I can tell, you just end up moving this unsightly hack to the new class instead. But the value of not having to create new conditional statements to set either port (for external memory) or cpu_side (for gem5's internal caches) seems worth this bit of nastiness. - Curtis --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/#review5914 --- On Feb. 20, 2015, 6:47 p.m., Curtis Dunham wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- (Updated Feb. 20, 2015, 6:47 p.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
Re: [gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- 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
[gem5-dev] Review Request 2668: config: Support full-system with SST's memory system
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2668/ --- 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