mem: add retry mechanism for cache fills in classic cache model

Information
Submitter:	Xiangyu Dong
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 9819:1f65fc6d046a
---------------------------
mem: add retry mechanism for cache fills in classic cache model
The changeset 6122d201ff80 modeled the cache bank and blocks the cache access
if the target bank is busy.  However, due to the lack of retry mechanism at the
cache master port, that changeset cannot properly blocks the cache traffic that
is towards CPU.
This patch modifies the CoherentBus model and adds a flow control scheme to the
RespLayer.  With this modification, cache fill operations can now be properly
modeled.
@todo The modification to CoherentBus is a little hacky. e.g. The recvRetry
functions for Req and Resp are not symmetric
@todo Might also need to modify noncoherent bus
@todo There is no write buffer entries for cache fill operations. An
      incremental patch will be needed to model the cache fill buffer

Testing Done

Issue Summary

Description	From	Last Updated	Status
I am a little concerned here as I am not sure what will happen first: the bank being marked free ...	Andreas Hansson	May 30, 2013, 3:44 p.m.	Resolved
I'm not thrilled at how it's done at the moment, but I can see the need for something similar. I ...	Andreas Hansson	May 30, 2013, 3:43 p.m.	Resolved
As above, in general this looks like the right way forward. I have some major reshuffling of this in the ...	Andreas Hansson	May 30, 2013, 3:43 p.m.	Resolved

Overall this looks like it's going in the right direction. I have some concerns and issue we should resolve, but overall it's a nice addition to the current functionality.

Xiangyu Dong April 19, 2013, 6:34 a.m. (April 19, 2013, 6:34 a.m.)

Thank you very much for the comment!  I will delay this patch until the changesets for new bus model are pushed to the main repo.

src/mem/cache/cache_impl.hh (Diff revision 1)

I am a little concerned here as I am not sure what will happen first: the bank being marked free or the retry sent?

Ultimately I would suggest to complicate things a little bit further and set a flag here (mustSendRespRetry or similar) and then in the event that unblocks the bank (is there one?) send out the retry once everything else is done.

Show all issues

Xiangyu Dong April 19, 2013, 6:34 a.m. (April 19, 2013, 6:34 a.m.)

I agree. I also feel here is some approximation. But, it should be removed if later a dedicated bank free event is introduced.

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

I'm not thrilled at how it's done at the moment, but I can see the need for something similar.

I have quite some bus patches in the pipeline, I'll try and get them on the reviewboard early next week.

Show all issues

Xiangyu Dong April 19, 2013, 6:34 a.m. (April 19, 2013, 6:34 a.m.)
```
before the bus patch release, I will leave it as is.
```

Andreas Hansson April 19, 2013, 9:44 p.m. (April 19, 2013, 9:44 p.m.)

Great, I'll try and get those patches on the board next week.

Thanks again for the very useful additions to the cache model.

Xiangyu Dong May 30, 2013, 3:43 p.m. (May 30, 2013, 3:43 p.m.)

Hi Andreas.  I've updated this changeset to work with your new multi-layer bus model.

src/mem/coherent_bus.cc (Diff revision 1)

As above, in general this looks like the right way forward.

I have some major reshuffling of this in the pipeline, making the buses properly multi-layered and accounting for their own time, so I'd like to wait with adding this functionality until that is done if that's ok.

Show all issues

Xiangyu Dong April 19, 2013, 6:34 a.m. (April 19, 2013, 6:34 a.m.)
```
as above
```

Diff:

Revision 2 (+51 -2)

Show changes

	src/mem/coherent_bus.hh
	src/mem/coherent_bus.cc
	src/mem/cache/base.hh
	src/mem/cache/cache_impl.hh

Diff:

Revision 3 (+56 -2)

Show changes

	src/mem/coherent_bus.hh
	src/mem/coherent_bus.cc
	src/mem/cache/base.hh
	src/mem/cache/cache_impl.hh

Description:

~		Changeset 9639:2aae67c49743
	~	Changeset 9819:82757d5512bb

		mem: add retry mechanism for cache fills in classic cache model
		The changeset 6122d201ff80 modeled the cache bank and blocks the cache access
		if the target bank is busy. However, due to the lack of retry mechanism at the
		cache master port, that changeset cannot properly blocks the cache traffic that
		is towards CPU.
		This patch modifies the CoherentBus model and adds a flow control scheme to the
		RespLayer. With this modification, cache fill operations can now be properly
		modeled.
		@todo The modification to CoherentBus is a little hacky. e.g. The recvRetry
		functions for Req and Resp are not symmetric
		@todo Might also need to modify noncoherent bus
		@todo There is no write buffer entries for cache fill operations. An
		incremental patch will be needed to model the cache fill buffer

Diff:

Revision 4 (+56 -2)

Show changes

	src/mem/coherent_bus.hh
	src/mem/coherent_bus.cc
	src/mem/cache/base.hh
	src/mem/cache/cache_impl.hh

Description:

~		Changeset 9819:82757d5512bb
	~	Changeset 9819:1f65fc6d046a

		mem: add retry mechanism for cache fills in classic cache model
		The changeset 6122d201ff80 modeled the cache bank and blocks the cache access
		if the target bank is busy. However, due to the lack of retry mechanism at the
		cache master port, that changeset cannot properly blocks the cache traffic that
		is towards CPU.
		This patch modifies the CoherentBus model and adds a flow control scheme to the
		RespLayer. With this modification, cache fill operations can now be properly
		modeled.
		@todo The modification to CoherentBus is a little hacky. e.g. The recvRetry
		functions for Req and Resp are not symmetric
		@todo Might also need to modify noncoherent bus
		@todo There is no write buffer entries for cache fill operations. An
		incremental patch will be needed to model the cache fill buffer

Diff:

Revision 5 (+58 -20)

Show changes

	src/mem/coherent_bus.hh
	src/mem/coherent_bus.cc
	src/mem/cache/base.hh
	src/mem/cache/base.cc
	src/mem/cache/cache_impl.hh

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Issue Summary

Diff:

Diff:

Description:

Changeset 9639:2aae67c49743

Changeset 9819:82757d5512bb

Diff:

Description:

Changeset 9819:82757d5512bb

Changeset 9819:1f65fc6d046a

Diff: