----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/790/#review1433 -----------------------------------------------------------
Hi Andreas, First off, I want to second what Steve said. This patch is an impressive collection of modifications and add some nice features to ports. I have a few high-level questions about the planned functionality and implementation. Then below I have several minor questions and comments. Brad == High-level Questions == 1. The TLM2 4-phase scheme is: begin request, end request, begin response, and end response, correct? If so, do packets have to complete all four phases if they survive the begin request phase? As you may know, currently there are request packets that don't require a response. How do those packets fit into the 4-phase scheme? Also a related follow-on question: Does the 4-phase scheme have any assumptions when packets are deleted or who can delete them? For instance, can only the first or fourth phase delete a packet? Can only the master delete a packet? 2. Is it necessary to pass the MasterId enum to all the port receive functions? It would be great if that wasn't necessary. 3. Does it make sense to create a base 4-PhaseBus class that both the non-coherent and coherent buses inherit from? It seems like there are a fair amount of code shared between the two. For example, currently the coherent bus is calling non-coherent bus functions. configs/common/FSConfig.py <http://reviews.m5sim.org/r/790/#comment1858> Just to be clear, this patch and the removal of all non-ALPHA FS system configuration would not be checked in as is, correct? I assume you did this just to point out that your changes currently only apply to ALPHA. src/cpu/base.hh <http://reviews.m5sim.org/r/790/#comment1861> src/cpu/simple/timing.hh <http://reviews.m5sim.org/r/790/#comment1860> What is preventing you from declaring master_id's type as the enum instead of an int? src/cpu/simple/timing.cc <http://reviews.m5sim.org/r/790/#comment1859> I like this change in function name. The new name is a much better description of what happens here. src/mem/CoherentBus.py <http://reviews.m5sim.org/r/790/#comment1862> src/mem/bridge.cc <http://reviews.m5sim.org/r/790/#comment1864> Be careful not to check this implementation of getBlockSize in? As I experienced firsthand, returning 0 will not cause funcitonal incorrectness, but does impact request behavior. - Brad On 2011-07-15 09:11:15, Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/790/ > ----------------------------------------------------------- > > (Updated 2011-07-15 09:11:15) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Changes to the gem5 memory-system (release-0.1) > > ------------------------------------------------------------------------------ > > What is the goal > > The goal is to make it easier to use gem5 for communication-centric > modelling by adopting a communication framework similar to that of the > TLM-2.0 transaction level modelling standard from the Open SystemC > Initiative (OSCI). Just like TLM-2.0, the basic idea behind the > changes is to facilitate modelling of inter-module communication > through a set of well-define module interfaces, e.g. a memory-mapped > interface, and a cache-maintenance interface. > > ------------------------------------------------------------------------------ > > What are the key improvements > > - Master and slave interfaces (SystemC sc_export), distinguishing the > different roles in inter-module communication. A master (TLM-2.0 > initiator) is a MemObject that initiates new transactions, and a > slave (TLM2 target) is a module that responds to transactions > initiated by master modules. The same module can act both as a > master and a slave, and this would typically be the case for a model > of a bridge, a router, or a bus. See src/mem/module_interface.hh > > - Master and slave port interfaces that are used to access the > corresponding module interfaces (SystemC sc_port). Together with the > module interfaces the port interfaces form the basis for having > ports and modules with different protocols. For example, a > memory-mapped master port would be used to call functions > implemented by a memory-mapped slave interface. See > src/mem/port_interface.{hh,cc} > > - Ports that only convey structural information and leave the syntax > (payload type) and semantics to a specific port interface. The > ports themselves only contain information about their owners > (structurally) and basic knowledge of their connectivity. The actual > semantics and syntax of the communication between a master and a > slave port is determined by their port interfaces and the > corresponding exported module interfaces. See src/mem/port.{hh,cc} > > - The specific type of packet is determined by the protocol of the > interfaces enabling diversity in payload types. See > src/mem/protocol.{hh,cc} and src/mem/packet.{hh,cc} > > - Standard 4-phase handshakes (TLM-2.0 approximately timed) for > request/response avoid complex receive/retry interaction between > modules in timing mode. Currently there is a mix, but the end goal > is to completely replace the old semantics. See > e.g. src/mem/noncoherent_bus_4phase.{hh,cc} > > - Ports do not implement any module-specific functionality but merely > calls functions on their interface classes. > > ------------------------------------------------------------------------------ > > How does it work > > A master port has a protocol-specific master-port interface, and is > associated with a master-module interface. An example of this is the > data and instruction port of a CPU. The master-port interface calls > functions on the connected slave-module interface. In the case of the > CPU, this could be e.g. memory, or a cache, both implementing the > memory-mapped slave interface and making it visible through a > memory-mapped slave port. The Python instantiation is currently based > on the position of the ports (left or right of the equality sign) to > determine if they are masters or slaves, but this should be eventually > be specified in the Module.py using subclasses of Port. Similarly, the > cache ports are currently determined by looking at the name of the > port. This should also be extended to use an enum or literal in the > Module.py. The binding is checked at instantiation time when the C++ > objects are created and their ports connected. As part of the > instantiation a structural diagram of the system is also created as a > Graphviz dot file. Run it through "dot -Tsvg" and have a look in your > browser. The diagram clearly shows what ports are connected, what > protocol and role they have, and as a tooltip if also shows the > address ranges of all memory-mapped slave ports. > > Currently the 4-phase handshake is used by the devices on the I/O bus, > including both slaves (e.g. UART) and masters (e.g. disks). A specific > bridge is used to convert between the different semantics. See > src/mem/bridge_classic_to_4phase.{hh,cc} and > src/mem/bridge_4phase_to_classic.{hh,cc}. The 4-phase is to completely > replace the old receive/retry handshakes, but this is still work in > progress. Until that point the 4-phase to classic bridges are used to > enable a gradual transition. Once everything is updated these bridges > are to be removed. > > Similar to the classic gem5 memory-system, a packet points to a > request and its associated data. In the typical case, a memory-mapped > request packet is created by a master, such as a DMA or a CPU. Once > beginReq is called, the sending (master) module should not change the > packet until its response is returned through a beginResp (where > applicable). An intermediate component, such as a bus or cache, may > create forwarded cache-maintenance packets from the original > memory-mapped request. Thus, one request gives rise to a chain of > requests and responses. Currently the lifetime and rules governing > those packets (and their request and data pointers) is work in > progress (see e.g. coherent_bus.cc). The original request/response may > be deallocated before the snoop request/response and vice versa. Smart > pointers and reference counting might be a viable solution, or > alternatively a more intelligent "snoop controller" in the > busses. In addition, the different packets (currently memory-mapped > and cache-maintenance) should be stripped of as much common > functionality as possible, reducing the memory-mapped packets to a > bare essential. > > ------------------------------------------------------------------------------ > > What is the intention with this patch > > This patch is not showing all the changes made to the repository, but > rather trying to highlight the most important changes by including a > selected set of files and modules. The selection is done as follows: > > - The underlying infrastructure > o Module Interface (what does a master/slave have to provide) > o Port Interface (how is it accesses through the ports) > o Port (how are the structural ports and logical interfaces bound > together) > > - The basic building blocks > o MemObject (maintain collections of ports and do look ups of names) > o Packet Queue (similar to the Payload Event Queue in TLM2, and > closely related to the SimpleTimingPort) > > - The models themselves > o NonCoherentBus 4-phase (showcase the simplicity of not having any > cache maintenance and 4-phase handshakes) > o I/O Device (show a simple memory-mapped slave) > o PhysicalMemory (same as above) > o Bridge (show the benefits of the protocol separation and clear > port roles) > o CPU (demonstrate the shift from functionality in the ports to > interfaces of the modules) > o Bridge classic to/from 4-phase (highlighting the difference between > the semantics) > > - An example of their use > o Tsunami system (show the port connections and the structure) > > The goal is to get feedback and suggestions on anything from the > actual design and how it is implemented, to the coding style and code > comments. This is also an opportunity for everyone to influence and > steer the changes and the integration into the main gem5 > repository. With this first review we also hope to share the remaining > trajectory for integration into the repository, chopping the > contributions up in incremental patches. > > ------------------------------------------------------------------------------ > > Testing and verification > > In order to work effectively we have limited the regression to only > include the quick Alpha tests. For these tests, the appropriate > updates have been made to connect the additional ports, and define the > role and protocol for the ports in question. Due to the changes in > timing, small deviations (plus minus a few percent) in statistics have > been observed for a number of tests. We have considered this to be > within reasonable limits and updated the reference behaviour. > > > Diffs > ----- > > configs/common/FSConfig.py 2a04edb07407 > src/cpu/BaseCPU.py 2a04edb07407 > src/cpu/base.hh 2a04edb07407 > src/cpu/base.cc 2a04edb07407 > src/cpu/o3/O3CPU.py 2a04edb07407 > src/cpu/o3/cpu.hh 2a04edb07407 > src/cpu/o3/cpu.cc 2a04edb07407 > src/cpu/simple/TimingSimpleCPU.py 2a04edb07407 > src/cpu/simple/timing.hh 2a04edb07407 > src/cpu/simple/timing.cc 2a04edb07407 > src/dev/Device.py 2a04edb07407 > src/dev/io_device.hh 2a04edb07407 > src/dev/io_device.cc 2a04edb07407 > src/dev/pcidev.cc 2a04edb07407 > src/mem/Bridge.py 2a04edb07407 > src/mem/Bridge4PhaseToClassic.py PRE-CREATION > src/mem/BridgeClassicTo4Phase.py PRE-CREATION > src/mem/CoherentBus.py PRE-CREATION > src/mem/NonCoherentBus4Phase.py PRE-CREATION > src/mem/bridge.hh 2a04edb07407 > src/mem/bridge.cc 2a04edb07407 > src/mem/bridge_4phase_to_classic.hh PRE-CREATION > src/mem/bridge_4phase_to_classic.cc PRE-CREATION > src/mem/bridge_classic_to_4phase.hh PRE-CREATION > src/mem/bridge_classic_to_4phase.cc PRE-CREATION > src/mem/coherent_bus.hh PRE-CREATION > src/mem/coherent_bus.cc PRE-CREATION > src/mem/mem_object.hh 2a04edb07407 > src/mem/mem_object.cc 2a04edb07407 > src/mem/module_interface.hh PRE-CREATION > src/mem/noncoherent_bus_4phase.hh PRE-CREATION > src/mem/noncoherent_bus_4phase.cc PRE-CREATION > src/mem/packet_queue.hh PRE-CREATION > src/mem/packet_queue.cc PRE-CREATION > src/mem/port.hh 2a04edb07407 > src/mem/port.cc 2a04edb07407 > src/mem/port_interface.hh PRE-CREATION > src/mem/port_interface.cc PRE-CREATION > tests/configs/tsunami-simple-timing.py 2a04edb07407 > > Diff: http://reviews.m5sim.org/r/790/diff > > > Testing > ------- > > > Screenshots > ----------- > > SVG structural diagram > http://reviews.m5sim.org/r/790/s/2/ > > > Thanks, > > Andreas > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
