misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/10]

Information
Submitter:	Christian Menard
Repository:	gem5
Branch:	default
Bugs:
Depends On:	~~3527~~
Blocks:	3686
Reviewers
Groups:	Default
People:

Description

Changeset 11795:321da7e75035
---------------------------
misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/10]

The current TLM bridge only provides a Slave Port that allows the gem5 world to
send request to the SystemC world. This patch series refractors and cleans up
the existing code, and adds a Master Port that allows the SystemC world to send
requests to the gem5 world.

This patch:
* Add the Master Port.
* Add an example application that isslustrates its use.

Testing Done

A simple example application consisting of a TLM traffic generator and a gem5 memory is part of the patch.

Issue Summary

Description	From	Last Updated	Status
Same as previous patch. This should use pkt-config.	Andreas Hansson	Oct. 28, 2016, 9:07 a.m.	Resolved
An assignment to a name would be good for these paths. GEM5_ROOT or similar	Andreas Hansson	Oct. 28, 2016, 9:07 a.m.	Resolved
gem5 has a random_mt that you should preferably use	Andreas Hansson	Oct. 28, 2016, 9:43 a.m.	Resolved
Same as the other patch. Why not use the response string size?	Andreas Hansson	Oct. 28, 2016, 9:10 a.m.	Resolved
Not convention	Andreas Hansson	Oct. 28, 2016, 8:47 a.m.	Resolved
Could you provide a bit more comments here around what is assumed, how it does what it does, what it ...	Andreas Hansson	Oct. 28, 2016, 9:28 a.m.	Resolved
Same as the header	Andreas Hansson	Oct. 28, 2016, 8:47 a.m.	Resolved
Is it assumed that the request is deleted elsewhere?	Andreas Hansson	Nov. 15, 2016, 3:10 p.m.	Dropped
comma on the line before name, other name	Andreas Hansson	Oct. 28, 2016, 8:54 a.m.	Resolved
Why the re-interpret cast?	Andreas Hansson	Oct. 28, 2016, 9:17 a.m.	Resolved
it is not a big problem, but technically gem5 can change mode during the simulation. The API for this is ...	Andreas Hansson	Oct. 28, 2016, 9:17 a.m.	Resolved
could we base this on the response string rather to avoid buffer overflows? :-)	Andreas Hansson	Oct. 28, 2016, 9:04 a.m.	Resolved
odd indentation	Andreas Hansson	Oct. 28, 2016, 9:05 a.m.	Resolved
what is this checking?	Andreas Hansson	Oct. 28, 2016, 9:14 a.m.	Resolved
This function is also in the target example, could we source out all doublicated functions to a common dir/file?	Matthias Jung	Nov. 7, 2016, 8:46 a.m.	Resolved
Similar to the Target (the example TLM memory): put this sc_module in a seperated file...	Matthias Jung	Nov. 7, 2016, 7:16 a.m.	Resolved
remove it if you dont need it	Matthias Jung	Nov. 7, 2016, 7:16 a.m.	Resolved
XXX flag?	Matthias Jung	Nov. 7, 2016, 7:16 a.m.	Resolved
It would be good to add a comment explaining where the request is deleted.	Andreas Hansson	Jan. 9, 2017, 4:24 a.m.	Resolved
this is not a safe assumption use SimClock::Int::ps for the "conversion"	Andreas Hansson	Jan. 9, 2017, 4:24 a.m.	Resolved
do things work even if this is not TLM_UPDATED?	Andreas Hansson	Jan. 9, 2017, 4:24 a.m.	Resolved

Dear Christian,

thank you for this nice contribution. I will have a look on your patch and maybe I can help you fixing your issue.

Regards
Matthias

Andreas Hansson July 1, 2016, 2 a.m. (July 1, 2016, 2 a.m.)

Christian, Matthias, is there any sensible way we could actually get some of this functionality tested? Including SystemC in ext seems problematic due to the non-BSD compatible license, but perhaps we should do something like we do with protobuf, and check if the host HAS_SYSTEMC or similar, and if so compile the extra bits, and run the extra tests etc.

This is tremendously useful functionality, and it feels like it is bolted on rather than properly integrated at the moment (not because of the functionality, but due to the non-default use). Any thoughts or suggestions?

Matthias Jung July 1, 2016, 10:26 a.m. (July 1, 2016, 10:26 a.m.)

What about an automated shell script that downloads SystemC from http://accellera.org/images/downloads/standards/systemc/systemc-2.3.1.tgz and compiles it? Its maybe a little complicated but makes things easy for people that don't have SystemC by default. Usually people install SystemC in /opt/systemc so thats a place that could be checked for existance.
Christian? what do you think?

Andreas Hansson July 7, 2016, 12:55 a.m. (July 7, 2016, 12:55 a.m.)

I don't like the idea of the download, and would rather suggest we should rely on pkg-config. Supposedly SystemC 2.3.1 actually does support pkg-config properly, with package names SystemC, TLM and TLM2. Could someone confirm? If that is the case, we could use the same flow as for protobuf for the determination of HAVE_SYSTEMC.
The next step would be to build in support if SystemC is present and look in more detail at how we deal with "with python" and building of the gem5 lib. Perhaps we want to revisit this flow and not mandate going via the lib for SystemC (and rather just use a different main.cc)?

Matthias Jung July 7, 2016, 1:08 a.m. (July 7, 2016, 1:08 a.m.)

find .  -iname *.pc
/opt/systemc/lib-macosx64/pkgconfig/systemc.pc
/opt/systemc/lib-macosx64/pkgconfig/tlm.pc

seems that pkg-config is used.

Change Summary:

Rebase patch to #3477

Depends On:

+	~~3477 - misc: SystemC Elastic Trace Player Example~~

Diff:

Revision 2 (+893 -10)

Show changes

	util/tlm/README
	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/Makefile
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/slave_port/Makefile

Change Summary:

Bugfix: Before this fix the test eventually resulted in an "Missed an event" error. With this new patch version, the event loop gets notified whenever a gem5 event is scheduled from a function in the SystemC world.
The review board complains about the patch not applying cleanly. Sorry about this, I tried to fix the patch, but locally it works perfectly...

Description:

		The current TLM code only provides a Slave Port that allows the gem5 world to send requests to the the TLM world. This patch adds a Master Port that allows the TLM world to send requests to the gem5 world. Furthermore, the patch provides a simple example application based on a TLM traffic generator.
-
-		As of now the example code does not run error free. It eventually results in an erro "fatal: Missed an event at time ...". So for now I leave the patch for discussion until the error is resolved.

Testing Done:

~		A simple example application consisting of a TLM traffic generator and a gem5 memory. Currently the test fails.
	~	A simple example application consisting of a TLM traffic generator and a gem5 memory is part of the patch.

Depends On:

-	~~3477 - misc: SystemC Elastic Trace Player Example~~
+	~~3480 - misc: add a MasterId to the ExternalPort~~

Diff:

Revision 3 (+930 -10)

Show changes

	util/tlm/README
	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/Makefile
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/slave_port/Makefile

Change Summary:

Updated the diff: rebased on 11691 and generated with mq

Diff:

Revision 4 (+930 -10)

Show changes

	util/tlm/README
	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/Makefile
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/slave_port/Makefile

Some of the files are not displaying ok. Did you post the review using hg postreview?

util/tlm/examples/master_port/Makefile (Diff revision 4)

Same as previous patch. This should use pkt-config.

Show all issues

util/tlm/examples/master_port/Makefile (Diff revision 4)

An assignment to a name would be good for these paths. GEM5_ROOT or similar

Show all issues

util/tlm/examples/master_port/main.cc (Diff revision 4)

gem5 has a random_mt that you should preferably use

Show all issues

util/tlm/examples/master_port/main.cc (Diff revision 4)

Same as the other patch. Why not use the response string size?

Show all issues

util/tlm/sc_master_port.hh (Diff revision 4)

Not convention

Show all issues

util/tlm/sc_master_port.hh (Diff revision 4)

Could you provide a bit more comments here around what is assumed, how it does what it does, what it doesn't do, etc

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

Same as the header

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

Is it assumed that the request is deleted elsewhere?

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

comma on the line before

name,
other name

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

Why the re-interpret cast?

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

it is not a big problem, but technically gem5 can change mode during the simulation.

The API for this is involving calls to drain and drain resume etc. We don't have to fix it now, but it may be worth at least warning if any of these functions are called.

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

could we base this on the response string rather to avoid buffer overflows? :-)

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

odd indentation

Show all issues

util/tlm/sc_master_port.cc (Diff revision 4)

what is this checking?

Show all issues

Summary:

-	misc: add a TLM to Gem5 Master Port implementation
+	misc: add a TLM to Gem5 Master Port

Description:

~		The current TLM code only provides a Slave Port that allows the gem5 world to send requests to the the TLM world. This patch adds a Master Port that allows the TLM world to send requests to the gem5 world. Furthermore, the patch provides a simple example application based on a TLM traffic generator.
	~	Changeset 11690:b9fc391b2477
	+	---------------------------
	+	misc: add a TLM to Gem5 Master Port
	+
	+	The current TLM code only provides a Slave Port that allows the gem5 world to
	+	send requests to the the TLM world. This patch adds a Master Port that allows
	+	the TLM world to send requests to the gem5 world. Furthermore, the patch
	+	provides a simple example application based on a TLM traffic generator.

Branch:

+	default

Diff:

Revision 5 (+921 -7)

Show changes

	util/tlm/README
	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/SConstruct
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py

util/tlm/examples/master_port/main.cc (Diff revision 5)

This function is also in the target example, could we source out all doublicated functions to a common dir/file?

Show all issues

util/tlm/examples/master_port/main.cc (Diff revision 5)

Similar to the Target (the example TLM memory): put this sc_module in a seperated file...

Show all issues

util/tlm/examples/master_port/tlm.py (Diff revision 5)

remove it if you dont need it

Show all issues

util/tlm/sc_master_port.cc (Diff revision 5)

XXX flag?

Show all issues

Description:

~		Changeset 11690:b9fc391b2477
	~	Changeset 11707:a9ccba85c073
		---------------------------
		misc: add a TLM to Gem5 Master Port

		The current TLM code only provides a Slave Port that allows the gem5 world to
		send requests to the the TLM world. This patch adds a Master Port that allows
		the TLM world to send requests to the gem5 world. Furthermore, the patch
		provides a simple example application based on a TLM traffic generator.

Diff:

Revision 6 (+1010 -7)

Show changes

	util/tlm/README
	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/SConstruct
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/master_port/traffic_generator.hh
	1 more

Description:

~		Changeset 11707:a9ccba85c073
	~	Changeset 11707:cb37987f081e
		---------------------------
		misc: add a TLM to Gem5 Master Port

		The current TLM code only provides a Slave Port that allows the gem5 world to
		send requests to the the TLM world. This patch adds a Master Port that allows
		the TLM world to send requests to the gem5 world. Furthermore, the patch
		provides a simple example application based on a TLM traffic generator.

Diff:

Revision 7 (+1013 -7)

Show changes

	util/tlm/README
	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/SConstruct
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/master_port/traffic_generator.hh
	1 more

Thanks for this. Some minor requests and style issues, then it's good to go.

util/tlm/sc_master_port.cc (Diff revision 7)

It would be good to add a comment explaining where the request is deleted.

Show all issues

util/tlm/sc_master_port.cc (Diff revision 7)

this is not a safe assumption

use SimClock::Int::ps for the "conversion"

Show all issues

util/tlm/sc_master_port.cc (Diff revision 7)

do things work even if this is not TLM_UPDATED?

Show all issues

Summary:

-	misc: add a TLM to Gem5 Master Port
+	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]

Description:

~		Changeset 11707:cb37987f081e
	~	Changeset 11795:bba360bcb0d0
		---------------------------
~		misc: add a TLM to Gem5 Master Port
	~	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]

~		The current TLM code only provides a Slave Port that allows the gem5 world to
~		send requests to the the TLM world. This patch adds a Master Port that allows
~		the TLM world to send requests to the gem5 world. Furthermore, the patch
~		provides a simple example application based on a TLM traffic generator.
	~	The current TLM bridge only provides a Slave Port that allows the gem5 world to
	~	send request to the SystemC world. This patch series refractors and cleans up
	~	the existing code, and adds a Master Port that allows the SystemC world to send
	~	requests to the gem5 world.
	+
	+	This patch:
	+	* Add the Master Port.
	+	* Add an example application that isslustrates its use.

Diff:

Revision 8 (+982)

Show changes

	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/SConstruct
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/master_port/traffic_generator.hh
	util/tlm/examples/master_port/traffic_generator.cc

Description:

~		Changeset 11795:bba360bcb0d0
	~	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]
-		---------------------------
-		misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]

		The current TLM bridge only provides a Slave Port that allows the gem5 world to
		send request to the SystemC world. This patch series refractors and cleans up
		the existing code, and adds a Master Port that allows the SystemC world to send
		requests to the gem5 world.

		This patch:
		* Add the Master Port.
		* Add an example application that isslustrates its use.

Depends On:

-	~~3480 - misc: add a MasterId to the ExternalPort~~

Description:

~		misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]
	~	Changeset 11795:13e84eff54ec
	+	---------------------------
	+	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]

		The current TLM bridge only provides a Slave Port that allows the gem5 world to
		send request to the SystemC world. This patch series refractors and cleans up
		the existing code, and adds a Master Port that allows the SystemC world to send
		requests to the gem5 world.

		This patch:
		* Add the Master Port.
		* Add an example application that isslustrates its use.

Diff:

Revision 9 (+982)

Show changes

	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/SConstruct
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/master_port/traffic_generator.hh
	util/tlm/examples/master_port/traffic_generator.cc

Description:

~		Changeset 11795:13e84eff54ec
	~	Changeset 11795:321da7e75035
		---------------------------
		misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]

		The current TLM bridge only provides a Slave Port that allows the gem5 world to
		send request to the SystemC world. This patch series refractors and cleans up
		the existing code, and adds a Master Port that allows the SystemC world to send
		requests to the gem5 world.

		This patch:
		* Add the Master Port.
		* Add an example application that isslustrates its use.

Diff:

Revision 10 (+982)

Show changes

	util/tlm/sc_master_port.hh
	util/tlm/sc_master_port.cc
	util/tlm/sim_control.cc
	util/tlm/examples/master_port/SConstruct
	util/tlm/examples/master_port/main.cc
	util/tlm/examples/master_port/tlm.py
	util/tlm/examples/master_port/traffic_generator.hh
	util/tlm/examples/master_port/traffic_generator.cc

Ship It!

Summary:

-	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]
+	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/10]

Description:

		Changeset 11795:321da7e75035
		---------------------------
~		misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/6]
	~	misc: Clean up and complete the gem5<->SystemC-TLM bridge [2/10]

		The current TLM bridge only provides a Slave Port that allows the gem5 world to
		send request to the SystemC world. This patch series refractors and cleans up
		the existing code, and adds a Master Port that allows the SystemC world to send
		requests to the gem5 world.

		This patch:
		* Add the Master Port.
		* Add an example application that isslustrates its use.

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Change Summary:

Depends On:

Diff:

Change Summary:

Description:

Testing Done:

Depends On:

Diff:

Change Summary:

Diff:

Summary:

Description:

Branch:

Diff:

Description:

Diff:

Description:

Diff:

Summary:

Description:

Diff:

Description:

Depends On:

Description:

Diff:

Description:

Diff:

Summary:

Description:

Status: Closed (submitted)