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

Information
Submitter:	Christian Menard
Repository:	gem5
Branch:	default
Bugs:
Depends On:	~~3480~~, ~~3807~~, ~~3478~~
Blocks:	3528
Reviewers
Groups:	Default
People:

Description

Changeset 11794:3b95cfab373f
---------------------------
misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/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:
* Restructure the existing sources in preparation of the addition of the new
Master Port.
* Refractor names to allow for distinction of the slave and master port.
* Replace the Makefile by a SConstruct.

Testing Done

The examples provided in util/tlm (now util/tlm/examples/slave_port) still compile and run error free.

Issue Summary

Description	From	Last Updated	Status
Just add the additional Copyright line before "All rights reserved."	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
Please avoid changing whitespace (even if it is wrong). That should be done separately.	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
Just leave Authors on the top line and indent your name.	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
Both of these lines should use pkg-config. I would suggest to do that as a separate patch though, and do ...	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
could you make a define for the directory rather?	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
c++11	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
This is a bit dubious, is it not?	Andreas Hansson	Oct. 28, 2016, 4:03 a.m.	Resolved
Same as before.	Andreas Hansson	Oct. 28, 2016, 4:29 a.m.	Resolved
Same as before	Andreas Hansson	Oct. 28, 2016, 4:04 a.m.	Resolved
keep the comma on the first line name, other name, yet another name	Andreas Hansson	Oct. 28, 2016, 4:08 a.m.	Resolved
spurious change	Andreas Hansson	Oct. 28, 2016, 4:09 a.m.	Resolved
fits on the line? less than 78 char?	Andreas Hansson	Oct. 28, 2016, 4:11 a.m.	Resolved
again, spurious change	Andreas Hansson	Oct. 28, 2016, 4:11 a.m.	Resolved
i'm not sure why this has changed	Andreas Hansson	Oct. 28, 2016, 4:11 a.m.	Resolved
ame here	Andreas Hansson	Oct. 28, 2016, 4:11 a.m.	Resolved
yet another one	Andreas Hansson	Oct. 28, 2016, 4:12 a.m.	Resolved
Same as before	Andreas Hansson	Oct. 28, 2016, 4:14 a.m.	Resolved
Great. Could you provide a bit more comments though. At the moment it is difficult to grasp.	Andreas Hansson	Oct. 28, 2016, 4:19 a.m.	Resolved
Same as before	Andreas Hansson	Oct. 28, 2016, 4:20 a.m.	Resolved
You may want to call this SCSlavePort or TLMSlavePort or similar to avoid confusion.	Andreas Hansson	Oct. 28, 2016, 4:28 a.m.	Resolved
Same as before	Andreas Hansson	Oct. 28, 2016, 4:29 a.m.	Resolved
Both directions are implemented, Initiator (Master) and Target (Slave)	Matthias Jung	Jan. 9, 2017, 4:21 a.m.	Resolved

please build your patch on top of this: http://reviews.gem5.org/r/3477/
I hope that this patch will be comitted to gem5 soon since it got already a lot of ship its.

So we will avoid merge problems ;)

Change Summary:

Rebase patch on #3477, patch does not apply cleanly anymor because of missing changes from #3477, selecting #3477 as parrent diff resulted in errors...

Depends On:

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

Diff:

Revision 2 (+1810 -1632)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/README
	util/tlm/main.cc
	util/tlm/payload_event.hh
	util/tlm/run_gem5.sh
	util/tlm/sc_port.hh
	util/tlm/sc_port.cc
	17 more

Change Summary:

Rebase the patch on cdb94f2332a6 and respect changes from d2e815b98287

Diff:

Revision 3 (+436 -250)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/README
	util/tlm/main.cc
	util/tlm/payload_event.hh
	util/tlm/run_gem5.sh
	util/tlm/sc_port.hh
	util/tlm/sc_port.cc
	7 more

Change Summary:

Fix the patch (It didn't apply cleanly)

Depends On:

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

Diff:

Revision 4 (+430 -244)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/README
	util/tlm/main.cc
	util/tlm/payload_event.hh
	util/tlm/run_gem5.sh
	util/tlm/sc_port.hh
	util/tlm/sc_port.cc
	7 more

Ship It!

Change Summary:

Updated the diff: rebased on 11691 and generated with mq

Diff:

Revision 5 (+436 -250)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/README
	util/tlm/main.cc
	util/tlm/payload_event.hh
	util/tlm/run_gem5.sh
	util/tlm/sc_port.hh
	util/tlm/sc_port.cc
	7 more

Thanks for this. There are a few minor issues and style questions.

util/tlm/examples/slave_port/Makefile (Diff revision 5)

Show all issues

util/tlm/examples/slave_port/Makefile (Diff revision 5)

Please avoid changing whitespace (even if it is wrong). That should be done separately.

Show all issues

util/tlm/examples/slave_port/Makefile (Diff revision 5)

Just leave Authors on the top line and indent your name.

Show all issues

util/tlm/examples/slave_port/Makefile (Diff revision 5)

Both of these lines should use pkg-config.

I would suggest to do that as a separate patch though, and do it before this patch.

Show all issues

util/tlm/examples/slave_port/Makefile (Diff revision 5)

could you make a define for the directory rather?

Show all issues

util/tlm/examples/slave_port/Makefile (Diff revision 5)

c++11

Show all issues

util/tlm/examples/slave_port/Makefile (Diff revision 5)

This is a bit dubious, is it not?

Show all issues

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

Same as before.

Show all issues

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

Same as before

Show all issues

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

keep the comma on the first line

name,
other name,
yet another name

Show all issues

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

spurious change

Show all issues

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

fits on the line? less than 78 char?

Show all issues

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

again, spurious change

Show all issues

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

i'm not sure why this has changed

Show all issues

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

ame here

Show all issues

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

yet another one

Show all issues

util/tlm/payload_event.hh (Diff revision 5)

Same as before

Show all issues

util/tlm/payload_event.hh (Diff revision 5)

Great. Could you provide a bit more comments though. At the moment it is difficult to grasp.

Show all issues

util/tlm/sc_slave_port.hh (Diff revision 5)

Same as before

Show all issues

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

You may want to call this SCSlavePort or TLMSlavePort or similar to avoid confusion.

Show all issues

util/tlm/sim_control.hh (Diff revision 5)

Same as before

Show all issues

Summary:

-	misc: prepare the TLM sources for the addition of a TLM->Gem5 Master Port
+	[mq]: 02_prepare.diff

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 restructures the existing source code in util/tlm in order to allow for code reuse and a clear file structure whenn adding a master port implementation.
	~	Changeset 11689:15cbc2f1136b
	+	---------------------------
	+	[mq]: 02_prepare.diff

Branch:

+	default

Diff:

Revision 6 (+1704 -1647)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/README
	util/tlm/main.cc
	util/tlm/run_gem5.sh
	util/tlm/sc_ext.hh
	util/tlm/sc_ext.cc
	util/tlm/sc_mm.hh
	20 more

Summary:

-	[mq]: 02_prepare.diff
+	misc: prepare the TLM sources for the addition of a TLM->Gem5 Master Port

Description:

~		Changeset 11689:15cbc2f1136b
	~	The current TLM code only provides a Slave Port that allows the gem5 world to send requests to the the TLM world. This patch restructures the existing source code in util/tlm in order to allow for code reuse and a clear file structure whenn adding a master port implementation. It also replaces the Makefile by a SConstruct.
-		---------------------------
-		[mq]: 02_prepare.diff

util/tlm/README (Diff revision 6)

Both directions are implemented, Initiator (Master) and Target (Slave)

Show all issues

Matthias Jung Oct. 28, 2016, 12:58 p.m. (Oct. 28, 2016, 12:58 p.m.)

Just add a little more information on the beginning of the readme

util/tlm/examples/slave_port/sc_target.cc (Diff revision 6)

once the patch is accepted i will change this to something like this:

unsigned int size = 1024 * 1024 * 1024;
        memory = (unsigned char *)mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON | MAP_NORESERVE, -1, 0);

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 restructures the existing source code in util/tlm in order to allow for code reuse and a clear file structure whenn adding a master port implementation. It also replaces the Makefile by a SConstruct.
	~	Changeset 11689:6cd8824bfe04
	+	---------------------------
	+	misc: prepare the TLM sources for the addition of a TLM->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 restructures the existing source
	+	code in util/tlm in order to allow for code reuse and a clear file structure
	+	whenn adding a master port implementation. It also replaces the Makefile by a
	+	SConstruct.

Diff:

Revision 7 (+1806 -1647)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/README
	util/tlm/main.cc
	util/tlm/run_gem5.sh
	util/tlm/sc_ext.hh
	util/tlm/sc_ext.cc
	util/tlm/sc_mm.hh
	21 more

Summary:

-	misc: prepare the TLM sources for the addition of a TLM->Gem5 Master Port
+	misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/6]

Description:

~		Changeset 11689:6cd8824bfe04
	~	Changeset 11794:75e8c8598373
		---------------------------
~		misc: prepare the TLM sources for the addition of a TLM->Gem5 Master Port
	~	misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/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 restructures the existing source
~		code in util/tlm in order to allow for code reuse and a clear file structure
~		whenn adding a master port implementation. It also replaces the Makefile by a
~		SConstruct.
	~	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:
	+	* Restructure the existing sources in preparation of the addition of the new
	+	Master Port.
	+	* Refractor names to allow for distinction of the slave and master port.
	+	* Replace the Makefile by a SConstruct.

Diff:

Revision 8 (+1795 -1634)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/main.cc
	util/tlm/run_gem5.sh
	util/tlm/sc_ext.hh
	util/tlm/sc_ext.cc
	util/tlm/sc_mm.hh
	util/tlm/sc_mm.cc
	20 more

Description:

~		Changeset 11794:75e8c8598373
	~	misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/6]
-		---------------------------
-		misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/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:
		* Restructure the existing sources in preparation of the addition of the new
		Master Port.
		* Refractor names to allow for distinction of the slave and master port.
		* Replace the Makefile by a SConstruct.

Depends On:

+	~~3480 - misc: add a MasterId to the ExternalPort~~
+	~~3774 - misc: Implement the Base SystemC Module as an sc_channel.~~
+	~~3478 - misc: Documentation Update~~

Description:

~		misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/6]
	~	Changeset 11794:3b95cfab373f
	+	---------------------------
	+	misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/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:
		* Restructure the existing sources in preparation of the addition of the new
		Master Port.
		* Refractor names to allow for distinction of the slave and master port.
		* Replace the Makefile by a SConstruct.

Diff:

Revision 9 (+1820 -1648)

Show changes

	configs/common/MemConfig.py
	util/tlm/Makefile
	util/tlm/main.cc
	util/tlm/run_gem5.sh
	util/tlm/sc_ext.hh
	util/tlm/sc_ext.cc
	util/tlm/sc_mm.hh
	util/tlm/sc_mm.cc
	20 more

Ship It!

Summary:

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

Description:

		Changeset 11794:3b95cfab373f
		---------------------------
~		misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/6]
	~	misc: Clean up and complete the gem5<->SystemC-TLM bridge [1/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:
		* Restructure the existing sources in preparation of the addition of the new
		Master Port.
		* Refractor names to allow for distinction of the slave and master port.
		* Replace the Makefile by a SConstruct.

Depends On:

-	~~3774 - misc: Implement the Base SystemC Module as an sc_channel.~~
+	~~3807 - misc: fix includes in util/systemc~~

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:

Diff:

Change Summary:

Depends On:

Diff:

Change Summary:

Diff:

Summary:

Description:

Branch:

Diff:

Summary:

Description:

Description:

Diff:

Summary:

Description:

Diff:

Description:

Depends On:

Description:

Diff:

Summary:

Description:

Depends On:

Status: Closed (submitted)