Port: Add protocol-agnostic ports in the port hierarchy

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

Description

Changeset 9125:c14aa7de9462
---------------------------
Port: Add protocol-agnostic ports in the port hierarchy

This patch adds an additional level of ports in the inheritance
hierarchy, separating out the protocol-specific and protocl-agnostic
parts. All the functionality related to the binding of ports is now
confined to use BaseMaster/BaseSlavePorts, and all the
protocol-specific parts stay in the Master/SlavePort. In the future it
will be possible to add other protocol-specific implementations.

The functions used in the binding of ports, i.e. getMaster/SlavePort
now use the base classes, and the index parameter is updated to use
the PortID typedef with the symbolic InvalidPortID as the default.

Testing Done

util/regress all passing (disregarding t1000 and eio)

Issue Summary

Description	From	Last Updated	Status
typo "overrive"	Steve Reinhardt	July 4, 2012, 5:47 p.m.	Resolved

src/cpu/testers/rubytest/RubyTester.cc (Diff revision 1)

Why this change? read_idx is an index, and not a Port ID.

Andreas Hansson June 19, 2012, 1:03 a.m. (June 19, 2012, 1:03 a.m.)

The getMasterPort and getSlavePort function is based on a Port identifier, and although plenty modules choose to store these in a vector and map them 1:1 to indices, that is not required.

I hope that makes sense.

Nilay Vaish June 19, 2012, 7:21 p.m. (June 19, 2012, 7:21 p.m.)
```
Then, read_idx should be declared as a PortID as well.
```

Andreas Hansson June 19, 2012, 11:44 p.m. (June 19, 2012, 11:44 p.m.)

Well, to clarify. read_idx should probably be a size_t to be in line with the STL containers. I will change this block accordingly. My statement about PortID was about the id in the getMaster/SlavePort.

src/mem/port.hh (Diff revision 1)

Should we have a virtual function for port binding here in the BaseSlavePort class as well?

Andreas Hansson June 19, 2012, 1:03 a.m. (June 19, 2012, 1:03 a.m.)

The point is to have the master being responsible for the binding, thus there is no need for such a member function on the slave side.

Hi Andreas,

This looks reasonable to me.  Before you commit it though, I'm curious about your longer-term vision for handling different protocols.  It's hard for me to feel completely confident in the naming and in the functionality split without a better idea of where you're headed.  For example, I'd think that sending and receiving are pretty basic operations as well, so should there be some form of send and receive in the base ports as well?  I can also see where if the packet class varies from protocol to protocol you don't want to hardwire that (but you could have a base packet class as well, which could be hardwired as the class for send/receive on the base ports).  Clearly there are a lot of ways to do it, and I don't know which one you have in mind.

This is probably something that's worth it's own email thread rather than doing it all in reviewboard comments...

Andreas Hansson July 4, 2012, 5:53 p.m. (July 4, 2012, 5:53 p.m.)

I'll post the follow-on patch within the next few days. That makes the port completely agnostic to the protocol, and uses a template class for the interface. Even the basic send/recv do not exist e.g. in the 4-phase begin/end Req/Resp handshakes, so the only think a port is left with is the connectivity to another port, and the responsibility for binding the interfaces of the two ports, everything else is left to the interface.

Once that patch is on the board we can kick off an e-mail thread.

src/mem/port.hh (Diff revision 1)

typo "overrive"

Show all issues

Andreas Hansson July 4, 2012, 5:53 p.m. (July 4, 2012, 5:53 p.m.)
```
Thanks. Well spotted :)
```

Description:

~		Changeset 9081:944fe8294fa3
	~	Changeset 9125:c14aa7de9462

		Port: Add protocol-agnostic ports in the port hierarchy

		This patch adds an additional level of ports in the inheritance
		hierarchy, separating out the protocol-specific and protocl-agnostic
		parts. All the functionality related to the binding of ports is now
		confined to use BaseMaster/BaseSlavePorts, and all the
		protocol-specific parts stay in the Master/SlavePort. In the future it
		will be possible to add other protocol-specific implementations.

		The functions used in the binding of ports, i.e. getMaster/SlavePort
		now use the base classes, and the index parameter is updated to use
		the PortID typedef with the symbolic InvalidPortID as the default.

Diff:

Revision 2 (+266 -164)

Show changes

	src/arch/arm/table_walker.hh
	src/arch/arm/table_walker.cc
	src/arch/arm/tlb.hh
	src/arch/arm/tlb.cc
	src/arch/x86/interrupts.hh
	src/arch/x86/pagetable_walker.hh
	src/arch/x86/pagetable_walker.cc
	src/arch/x86/tlb.hh
	39 more

Ship It!

Ship It!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Description:

Changeset 9081:944fe8294fa3

Changeset 9125:c14aa7de9462

Diff:

Status: Closed (submitted)