-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1223/#review2843
-----------------------------------------------------------

Ship it!


Looks OK to me.  I can see where a common BaseBus class would clean up the 
constructor situation a bit, and I think would allow some of the 
NoncoherentBus-only bits like the ports to be more cleanly separated.  If 
you're up for it, Andreas, you might want to give it a try.  It's not 
unreasonable as it stands though.

Comments below are orthogonal to the patch, just because I noticed this code 
for the first time.


src/mem/noncoherent_bus.cc
<http://reviews.gem5.org/r/1223/#comment3138>

    I see this code isn't new, but I never noticed it before... does anything 
actually work if connected devices don't all have the same block size?  I'm 
just concerned that this code gives the impression that there's more 
flexibility than actually exists.



src/mem/noncoherent_bus.cc
<http://reviews.gem5.org/r/1223/#comment3139>

    On the other hand, this warning seems overly conservative... other block 
sizes should work fine as long as *everyone* uses the same size.


- Steve Reinhardt


On May 25, 2012, 9:49 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1223/
> -----------------------------------------------------------
> 
> (Updated May 25, 2012, 9:49 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Changeset 9028:cd897893780f
> ---------------------------
> Bus: Split the bus into a non-coherent and coherent bus
> 
> This patch introduces a class hierarchy of buses, a non-coherent one,
> and a coherent one, splitting the existing bus functionality. By doing
> so it also enables further specialisation of the two types of buses.
> 
> A non-coherent bus connects a number of non-snooping masters and
> slaves, and routes the request and response packets based on the
> address. The request packets issued by the master connected to a
> non-coherent bus could still snoop in caches attached to a coherent
> bus, as is the case with the I/O bus and memory bus in most system
> configurations. No snoops will, however, reach any master on the
> non-coherent bus itself. The non-coherent bus can be used as a
> template for modelling PCI, PCIe, and non-coherent AMBA and OCP buses,
> and is typically used for the I/O buses.
> 
> A coherent bus connects a number of (potentially) snooping masters and
> slaves, and routes the request and response packets based on the
> address, and also forwards all requests to the snoopers and deals with
> the snoop responses. The coherent bus can be used as a template for
> modelling QPI, HyperTransport, ACE and coherent OCP buses, and is
> typically used for the L1-to-L2 buses and as the main system
> interconnect.
> 
> The configuration scripts are updated to use a NoncoherentBus for all
> peripheral and I/O buses.
> 
> A bit of minor tidying up has also been done.
> 
> 
> Diffs
> -----
> 
>   configs/common/CacheConfig.py bb25e7646c41 
>   configs/common/FSConfig.py bb25e7646c41 
>   configs/example/memtest.py bb25e7646c41 
>   configs/example/se.py bb25e7646c41 
>   configs/splash2/cluster.py bb25e7646c41 
>   configs/splash2/run.py bb25e7646c41 
>   src/cpu/BaseCPU.py bb25e7646c41 
>   src/mem/Bus.py bb25e7646c41 
>   src/mem/SConscript bb25e7646c41 
>   src/mem/bus.hh bb25e7646c41 
>   src/mem/bus.cc bb25e7646c41 
>   src/mem/coherent_bus.hh PRE-CREATION 
>   src/mem/coherent_bus.cc PRE-CREATION 
>   src/mem/noncoherent_bus.hh PRE-CREATION 
>   src/mem/noncoherent_bus.cc PRE-CREATION 
>   tests/configs/inorder-timing.py bb25e7646c41 
>   tests/configs/memtest.py bb25e7646c41 
>   tests/configs/o3-timing-checker.py bb25e7646c41 
>   tests/configs/o3-timing-mp-ruby.py bb25e7646c41 
>   tests/configs/o3-timing-mp.py bb25e7646c41 
>   tests/configs/o3-timing-ruby.py bb25e7646c41 
>   tests/configs/o3-timing.py bb25e7646c41 
>   tests/configs/pc-o3-timing.py bb25e7646c41 
>   tests/configs/pc-simple-atomic.py bb25e7646c41 
>   tests/configs/pc-simple-timing.py bb25e7646c41 
>   tests/configs/realview-o3-checker.py bb25e7646c41 
>   tests/configs/realview-o3-dual.py bb25e7646c41 
>   tests/configs/realview-o3.py bb25e7646c41 
>   tests/configs/realview-simple-atomic-dual.py bb25e7646c41 
>   tests/configs/realview-simple-atomic.py bb25e7646c41 
>   tests/configs/realview-simple-timing-dual.py bb25e7646c41 
>   tests/configs/realview-simple-timing.py bb25e7646c41 
>   tests/configs/simple-atomic-dummychecker.py bb25e7646c41 
>   tests/configs/simple-atomic-mp-ruby.py bb25e7646c41 
>   tests/configs/simple-atomic-mp.py bb25e7646c41 
>   tests/configs/simple-atomic.py bb25e7646c41 
>   tests/configs/simple-timing-mp.py bb25e7646c41 
>   tests/configs/simple-timing.py bb25e7646c41 
>   tests/configs/tsunami-inorder.py bb25e7646c41 
>   tests/configs/tsunami-o3-dual.py bb25e7646c41 
>   tests/configs/tsunami-o3.py bb25e7646c41 
>   tests/configs/tsunami-simple-atomic-dual.py bb25e7646c41 
>   tests/configs/tsunami-simple-atomic.py bb25e7646c41 
>   tests/configs/tsunami-simple-timing-dual.py bb25e7646c41 
>   tests/configs/tsunami-simple-timing.py bb25e7646c41 
> 
> Diff: http://reviews.gem5.org/r/1223/diff/
> 
> 
> Testing
> -------
> 
> util/regress all passing (disregarding t1000 and eio)
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to