(Switching from reviewboard to email as I fear this might be a more extended discussion. Not that I mind, if it helps us to do the right thing.)
Thanks for pointing out your port/interface separation patch... I had forgotten about that since it was so long ago that you posted it. I agree that my patch conflicts with what you did there, but I think more because we are both trying to do similar things (i.e., make ports more generic and independent of the things they connect) than really going in different directions... though we do go about it in different ways. There are a few additional details to work out, but based on your patch, it should be possible to implement Ethernet connections using Ports by doing something along the lines of IfaceMasterPort<EtherHandler, EtherHandler>. So I will play devil's advocate a bit and say that the problem with your patch is that it doesn't go far enough; it makes the concept of Port more generic, but unnecessarily constrains it to be used only in the memory system by leaving it tied to MemObject. If we push the concept of Port up to be something that's shared by all SimObjects, then we can use it for Ethernet and other types of connections as well as memory system connections, and then it would encompass a superset of what my patch enables as well. Thoughts? Steve On Thu, Jun 13, 2013 at 11:05 AM, Andreas Hansson <[email protected]>wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1922/ > > On June 13th, 2013, 8:33 a.m., *Andreas Hansson* wrote: > > You want to remove some ugly code in a portion that is only really done once > for most systems, and the "price" is a more complex object structure for the > ports. My gut feeling is that these different ports have little in common. > For example, their actual interfaces are very different. I also think this > patch makes the port/interface separation much more difficult. > > > On June 13th, 2013, 10:59 a.m., *Steve Reinhardt* wrote: > > Actually my motivation for doing this is that I want to introduce a third > type of port connection (for internal use only, for now), and scaling the > current approach to a third type really compounds the ugliness, particularly > because it requires modifying pyobject.cc directly (i.e., it can't be done > via EXTRAS). The advantage of this change is that it makes the port > connection paradigm independent of the type of ports, meaning it's easy to > add more types. Sorry for not explaining that up front. > > The different ports do indeed have little in common other than their method > of connection, but I contend that if they are going to share the same method > of connection, the new code is a much better way of doing that. Another > approach that I considered would be to forget using python assignment to bind > Ethernet devices and links. If we add the constraints that (1) all Ethernet > connections are made via point-to-point links and (2) those links are always > symmetric in delay and bandwidth, then we could just pass in an EtherLink > pointer as a param for each object that wants a connection. While I would be > fine with that, particularly if we had started out that way, switching to > that approach now both introduces more constraints and would require > modifications to the (admittedly few) scripts that use Ethernet links, so I > thought this approach was preferable. > > I'm not sure what you mean by "makes the port/interface separation much more > difficult"; to me, it makes the port concept more abstract, which should make > it more separable from other issues. > > The last statement was referring to the changes needed to support: > http://reviews.gem5.org/r/1301/. I fear this additional level of Port makes > that binding process more challenging. > > I definitely support the approach you mention with the EtherLink pointers. > Supposedly that also avoids the (in my opinion rather confusing) binding by =. > > > - Andreas > > On June 13th, 2013, 7:58 a.m., Steve Reinhardt wrote: > Review request for Default. > By Steve Reinhardt. > > *Updated June 13, 2013, 7:58 a.m.* > Description > > Changeset 9757:071a2d97327a > --------------------------- > sim: unify memory & ethernet port binding mechanisms. > > Get rid of the ugly special-casing and dynamic casting going > on in connectPorts(), and make the port binding mechanism > more extensible. This is done by moving the port lookup > methods to SimObject and creating a common base Port class > from which both the old Port (now MemPort) and EtherInt > (which perhaps should be renamed EtherPort) derive. > > Diffs > > - src/arch/x86/bios/acpi.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/dev/etherdevice.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/dev/etherdevice.cc (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/dev/etherint.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/dev/etherint.cc (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/dev/etherlink.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/dev/etherobject.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/mem/mem_object.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/mem/mem_object.cc (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/mem/port.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/mem/port.cc (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/python/swig/pyobject.cc > (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/sim/port.hh (PRE-CREATION) > - src/sim/sim_object.hh (0b4a08751b42ac522e296bd1a12408b816068fa1) > - src/sim/sim_object.cc (0b4a08751b42ac522e296bd1a12408b816068fa1) > > View Diff <http://reviews.gem5.org/r/1922/diff/> > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
