----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3204/#review7546 -----------------------------------------------------------
I didn't look through all the code closely, but it LGTM. Can you highlight in the commit message what new functionality this restructuring provides? (e.g., are the two bullets at the end that *don't* say "This behavior is the same as in the old implementation" a complete summary of the new functionality?) What are examples of devices that might want to override GenericPciHost? Also, now that we have a dev/pci directory, do you plan to move the other dev/pci*.* files there? (I agree that should be a separate changeset though.) src/dev/pci/host.hh (line 75) <http://reviews.gem5.org/r/3204/#comment6479> when did we start putting base classes on a separate line? not a huge deal, but an inconsistency wrt the rest of the code base - Steve Reinhardt On Nov. 12, 2015, 4:22 a.m., Andreas Sandberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3204/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2015, 4:22 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11206:c7d8a26d24a5 > --------------------------- > dev: Rewrite PCI host functionality > > The gem5's current PCI host functionality is very ad hoc. The current > implementations require PCI devices to be hooked up to the > configuration space via a separate configuration port. Devices query > the platform to get their config-space address range. Un-mapped parts > of the config space are intercepted using the XBar's default port > mechanism and a magic catch-all device (PciConfigAll). > > This changeset redesigns the PCI host functionality to improve code > reuse and make config-space and interrupt mapping more transparent. > > PCI devices now register themselves with a PCI host controller. The > host controller interface is defined in the abstract base class > PciHost. Registration is done by PciHost::registerDevice() which takes > the device, its bus position (bus/dev/func tuple), and its interrupt > pin (INTA-INTC) as a parameter. The registration interface returns a > PciHost::DeviceInterface that the PCI device can use to query memory > mappings and signal interrupts. > > The host device manages the entire PCI configuration space. Accesses > to devices decoded into the devices bus position and then forwarded to > the correct device. > > Basic PCI host functionality is implemented in the GenericPciHost base > class. Most platforms can use this class as a basic PCI controller. It > provides the following functionality: > > * Configurable configuration space decoding. The number of bits > dedicated to a device is a prameter, making it possible to support > both CAM and ECAM mappings. > > * Basic interrupt mapping using the interruptLine value from a > device's configuration space. This behavior is the same as in the > old implementation. > > * Simple (base + addr) remapping from the PCI bus's address space to > physical addresses for PIO, memory, and DMA. > > > Diffs > ----- > > src/dev/platform.cc 7a9eeecf2b52 > src/dev/x86/Pc.py 7a9eeecf2b52 > src/dev/pcidev.hh 7a9eeecf2b52 > src/dev/pcidev.cc 7a9eeecf2b52 > src/dev/platform.hh 7a9eeecf2b52 > src/dev/alpha/tsunami_pchip.cc 7a9eeecf2b52 > src/dev/arm/RealView.py 7a9eeecf2b52 > src/dev/arm/realview.hh 7a9eeecf2b52 > src/dev/arm/realview.cc 7a9eeecf2b52 > src/dev/pci/PciHost.py PRE-CREATION > src/dev/pci/SConscript PRE-CREATION > src/dev/pci/host.hh PRE-CREATION > src/dev/pci/host.cc PRE-CREATION > src/dev/pci/types.hh PRE-CREATION > src/dev/pciconfigall.hh 7a9eeecf2b52 > src/dev/pciconfigall.cc 7a9eeecf2b52 > configs/common/FSConfig.py 7a9eeecf2b52 > src/dev/Pci.py 7a9eeecf2b52 > src/dev/SConscript 7a9eeecf2b52 > src/dev/alpha/Tsunami.py 7a9eeecf2b52 > src/dev/alpha/tsunami.hh 7a9eeecf2b52 > src/dev/alpha/tsunami.cc 7a9eeecf2b52 > src/dev/alpha/tsunami_pchip.hh 7a9eeecf2b52 > src/dev/x86/SouthBridge.py 7a9eeecf2b52 > src/dev/x86/pc.hh 7a9eeecf2b52 > src/dev/x86/pc.cc 7a9eeecf2b52 > > Diff: http://reviews.gem5.org/r/3204/diff/ > > > Testing > ------- > > Quick regressions pass. Manually verified the diff between two checkpoints of > a booted system. > > > Thanks, > > Andreas Sandberg > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
