mem: Allow read-only caches and check compliance

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

Description

Changeset 10884:3c905b624d7e
---------------------------
mem: Allow read-only caches and check compliance

This patch adds a parameter to the BaseCache to enable a read-only
cache, for example for the instruction cache, or table-walker cache
(not for x86). A number of checks are put in place in the code to
ensure a read-only cache does not end up with dirty data.

A follow-on patch adds suitable read requests to allow a read-only
cache to explicitly ask for clean data.

Testing Done

Issue Summary

Description	From	Last Updated	Status
I and D caches got swapped.	Nilay Vaish	June 29, 2015, 2:25 a.m.	Resolved
Should be set to False by default	Nilay Vaish	June 29, 2015, 2:27 a.m.	Resolved
So should we not set this block to be writeable only for notReadOnly caches.	Nilay Vaish	June 29, 2015, 2:29 a.m.	Resolved

configs/common/CacheConfig.py (Diff revision 1)

I and D caches got swapped.

Show all issues

configs/common/Caches.py (Diff revision 1)

Should be set to False by default

Show all issues

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

So should we not set this block to be writeable only for notReadOnly caches.

Show all issues

Andreas Hansson June 29, 2015, 2:29 a.m. (June 29, 2015, 2:29 a.m.)

I have updated the comment to make it clear why we do this. In short it is because Exclusive = Valid | Readable | Writable.

Description:

~		Changeset 10874:0c3bee36e48e
	~	Changeset 10884:0de97ed85e6b

		mem: Allow read-only caches and check compliance

		This patch adds a parameter to the BaseCache to enable a read-only
		cache, for example for the instruction cache, or table-walker cache
		(not for x86). A number of checks are put in place in the code to
		ensure a read-only cache does not end up with dirty data.

		A follow-on patch adds suitable read requests to allow a read-only
		cache to explicitly ask for clean data.

Diff:

Revision 2 (+56 -12)

Show changes

	configs/common/CacheConfig.py
	configs/common/Caches.py
	configs/common/O3_ARM_v7a.py
	src/mem/cache/BaseCache.py
	src/mem/cache/base.hh
	src/mem/cache/base.cc
	src/mem/cache/cache_impl.hh
	tests/configs/base_config.py
	1 more

I don't have a problem with this, but I'm curious for a little more detail on the motivation---in particular, what's the rationale for using panic_if() instead of assert() (or chatty_assert()) in the checks?  Do we really need to have these checks in even in gem5.fast?

Andreas Hansson June 29, 2015, 8:13 p.m. (June 29, 2015, 8:13 p.m.)

I am happy to change the panic's to chatty_asserts.

There are two goals: 1) Capture a realistic behaviour, and 2) Be able to express and check basic assumptions. It turns out that for quite a few of the ISAs we actually had dirty data in the I cache for example, and it was a surprise to me that X86 actually writes into the TLB cache.

This patch also helps in facilitating the other cache improvements (ReadCleanReq vs ReadSharedReq, and the removal of is_top_level).

Summary:

-	mem: Allow read-only caches and check compliance
+	1;2c# HG changeset patch

Description:

~		Changeset 10884:0de97ed85e6b
	~	Changeset 10884:a49fc0709fb1
	+
	+	1;2c# HG changeset patch
	+
	+	Parent eaf39f841ee86bd48a5d0fd8a87f2601947a77a0
	+
	+	User Andreas Hansson andreas.hansson@arm.com

		mem: Allow read-only caches and check compliance

		This patch adds a parameter to the BaseCache to enable a read-only
		cache, for example for the instruction cache, or table-walker cache
		(not for x86). A number of checks are put in place in the code to
		ensure a read-only cache does not end up with dirty data.

		A follow-on patch adds suitable read requests to allow a read-only
		cache to explicitly ask for clean data.

Diff:

Revision 3 (+56 -12)

Show changes

	configs/common/CacheConfig.py
	configs/common/Caches.py
	configs/common/O3_ARM_v7a.py
	src/mem/cache/BaseCache.py
	src/mem/cache/base.hh
	src/mem/cache/base.cc
	src/mem/cache/cache_impl.hh
	tests/configs/base_config.py
	1 more

Summary:

-	1;2c# HG changeset patch
+	mem: Allow read-only caches and check compliance

Description:

~		Changeset 10884:a49fc0709fb1
	~	Changeset 10884:3c905b624d7e
-
-		1;2c# HG changeset patch
-
-		Parent eaf39f841ee86bd48a5d0fd8a87f2601947a77a0
-
-		User Andreas Hansson andreas.hansson@arm.com

		mem: Allow read-only caches and check compliance

		This patch adds a parameter to the BaseCache to enable a read-only
		cache, for example for the instruction cache, or table-walker cache
		(not for x86). A number of checks are put in place in the code to
		ensure a read-only cache does not end up with dirty data.

		A follow-on patch adds suitable read requests to allow a read-only
		cache to explicitly ask for clean data.

Diff:

Revision 4 (+56 -12)

Show changes

	configs/common/CacheConfig.py
	configs/common/Caches.py
	configs/common/O3_ARM_v7a.py
	src/mem/cache/BaseCache.py
	src/mem/cache/base.hh
	src/mem/cache/base.cc
	src/mem/cache/cache_impl.hh
	tests/configs/base_config.py
	1 more

I feel like chatty_assert() is kind of a goofy name, but that's a separate issue... (anyone for something short like "assertv()"?)

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Description:

Changeset 10874:0c3bee36e48e

Changeset 10884:0de97ed85e6b

Diff:

Summary:

Description:

Changeset 10884:0de97ed85e6b

Changeset 10884:a49fc0709fb1

Parent eaf39f841ee86bd48a5d0fd8a87f2601947a77a0

User Andreas Hansson andreas.hansson@arm.com

Diff:

Summary:

Description:

Changeset 10884:a49fc0709fb1

Changeset 10884:3c905b624d7e

Parent eaf39f841ee86bd48a5d0fd8a87f2601947a77a0

User Andreas Hansson andreas.hansson@arm.com

Diff:

Status: Closed (submitted)