Andreas Hansson got review request #1001!

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

MEM: Introduce the master/slave port roles in the Python classes

This patch classifies all ports in Python as either Master or Slave
and enforces a binding of master to slave. Conceptually, a master (such
as a CPU or DMA port) issues requests, and receives responses, and
conversely, a slave (such as a memory or a PIO device) receives
requests and sends back responses. Currently there is no
differentiation between coherent and non-coherent masters and slaves.

The classification as master/slave also involves splitting the dual
role port of the bus into a master and slave port and updating all the
system assembly scripts to use the appropriate port. Similarly, the
interrupt devices have to have their int_port split into a master and
slave port. The intdev and its children have minimal changes to
facilitate the extra port.

Note that this patch does not enforce any port typing in the C++
world, it merely ensures that the Python objects have a notion of the
port roles and are connected in an appropriate manner. This check is
carried when two ports are connected, e.g. bus.master =
memory.port. The following patches will make use of the
classifications and specialise the C++ ports into masters and slaves.

Testing Done

util/regress all passing (disregarding t1000 and eio)

Issue Summary

Description	From	Last Updated	Status
Is there still a need to export Port and VectorPort, or should they be internal-only classes? It's just a matter ...	Steve Reinhardt	Feb. 10, 2012, 1:35 a.m.	Resolved

seems fine to me. Steve?

Overall this is great.  My only concern is the syntax of declaring master vs. slave... the string keywords are a little verbose.  If we're eventually going to expand the set of strings to include the protocol (e.g., 'MOESICacheMaster' or something) then using strings is probably OK.  If the master/slave distinction is going to be maintained orthogonal to the protocol specification, I'd prefer to see us expand Port/VectorPort to be subclassed as MasterPort/SlavePort/VectorMasterPort/VectorSlavePort.

Andreas Hansson Feb. 3, 2012, 1:13 a.m. (Feb. 3, 2012, 1:13 a.m.)

The master slave notion is universal and should be orthogonal to the protocol. I'm happy to make the change to use subclasses. Do we have consensus on this?

Description:

		MEM: Introduce the master/slave port roles in the Python classes

		This patch classifies all ports in Python as either Master or Slave
~		and enfores a binding of master to slave. Conceptually, a master (such
	~	and enforces a binding of master to slave. Conceptually, a master (such
		as a CPU or DMA port) issues requests, and receives responses, and
		conversely, a slave (such as a memory or a PIO device) receives
		requests and sends back responses. Currently there is no
		differentiation between coherent and non-coherent masters and slaves.

		The classification as master/slave also involves splitting the dual
		role port of the bus into a master and slave port and updating all the
		system assembly scripts to use the appropriate port. Similarly, the
		interrupt devices have to have their int_port split into a master and
		slave port. The intdev and its children have minimal changes to
		facilitate the extra port.

~		Note that this patch does not enfore any port typing in the C++ world,
~		it merely ensures that the Python objects have a notion of the port
~		roles and are connected in an appropriate manner. This check is
	~	Note that this patch does not enforce any port typing in the C++
	~	world, it merely ensures that the Python objects have a notion of the
	~	port roles and are connected in an appropriate manner. This check is
		carried when two ports are connected, e.g. bus.master =
		memory.port. The following patches will make use of the
		classifications and specialise the C++ ports into masters and slaves.

Diff:

Revision 2 (+482 -361)

Show changes

	configs/common/CacheConfig.py
	configs/common/FSConfig.py
	configs/example/fs.py
	configs/example/ruby_fs.py
	configs/example/se.py
	configs/ruby/Ruby.py
	src/arch/arm/ArmTLB.py
	src/arch/x86/X86LocalApic.py
	56 more

src/python/m5/params.py (Diff revision 2)

Is there still a need to export Port and VectorPort, or should they be internal-only classes?  It's just a matter of whether they should be taken of this __all__ list or not.

Show all issues

Andreas Hansson Feb. 10, 2012, 1:46 a.m. (Feb. 10, 2012, 1:46 a.m.)

No need for VectorPort, now removed. Port, however, has to be exported to enable a test in SimObject:

...
elif isinstance(val, Port):
    cls._new_port(key, val)

Sounds good?

Andreas Hansson Feb. 10, 2012, 1:50 a.m. (Feb. 10, 2012, 1:50 a.m.)

I have removed Port as well from __all__ and instead rely on the following in SimObject.py:

# There are a few things we need that aren't in params.__all__ since
# normal users don't need them
from m5.params import ParamDesc, VectorParamDesc, \
     isNullPointer, SimObjectVector, Port

Andreas Hansson Feb. 12, 2012, 1:22 a.m. (Feb. 12, 2012, 1:22 a.m.)
```
Steve, are you happy with this?
```

Steve Reinhardt Feb. 12, 2012, 7:01 a.m. (Feb. 12, 2012, 7:01 a.m.)

Yes, in fact I was going to suggest something similar after your first response but then you beat me to it.  Sorry for not responding sooner.

Congrats on patch #1001... at least reviewboard seems to think that's worth celebrating!

You have a pending review.

Review Board 2.0.15

Andreas Hansson got review request #1001!

This change has been marked as submitted.

MEM: Introduce the master/slave port roles in the Python classes

Screenshots

Files

Issue Summary

Description:

Diff:

Status: Closed (submitted)