mtest: Disable dcache during test

PATCH New Benoît Thébaudeau 5 participants 13 messages Aug 10, 2012 – Sep 4, 2012

Share & tools

Benoît Thébaudeau

author

2012-08-10 19:16 UTC #0 lore raw

PATCH 1/1 mtest: Disable dcache during test

mtest is supposed to test many types of memory accesses in many different conditions. If dcache is enabled, memory accesses are likely bursts, and some memory accesses are simply skipped. Hence, dcache should be disabled during mtest operation so that what mtest actually tests is not masked by dcache.

Signed-off-by Beno?t Th?baudeau <[email protected]>

Cc Wolfgang Denk <[email protected]>

---
 .../common/cmd_mem.c                               |   39 ++++++++++++++------
 1 file changed, 27 insertions(+), 12 deletions(-)

common/cmd_mem.c +27 ~~-12~~

index 18f0a3f..5d2b735 100644
@@ -600,6 +600,8 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	ulong	errs = 0;
 	int iterations = 1;
 	int iteration_limit;
+	int dcache;
+	int ret = 1;
 
 #if defined(CONFIG_SYS_ALT_MEMTEST)
 	vu_long	len;

@@ -651,6 +653,13 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	else
 		iteration_limit = 0;
 
+	/* Perform tests on the underlying memory rather than on the D-cache. */
+	dcache = dcache_status();
+	if (dcache) {
+		dcache_disable();
+		invalidate_dcache_all();
+	}
+
 #if defined(CONFIG_SYS_ALT_MEMTEST)
 	printf ("Testing %08x ... %08x:\n", (uint)start, (uint)end);
 	debug("%s:%d: start 0x%p end 0x%p\n",

@@ -659,14 +668,15 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	for (;;) {
 		if (ctrlc()) {
 			putc ('\n');
-			return 1;
+			goto end;
 		}
 
 
 		if (iteration_limit && iterations > iteration_limit) {
 			printf("Tested %d iteration(s) with %lu errors.\n",
 				iterations-1, errs);
-			return errs != 0;
+			ret = errs != 0;
+			goto end;
 		}
 
 		printf("Iteration: %6d\r", iterations);

@@ -704,7 +714,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			    errs++;
 			    if (ctrlc()) {
 				putc ('\n');
-				return 1;
+				goto end;
 			    }
 			}
 			*addr  = ~val;

@@ -717,7 +727,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			    errs++;
 			    if (ctrlc()) {
 				putc ('\n');
-				return 1;
+				goto end;
 			    }
 			}
 		    }

@@ -787,7 +797,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			errs++;
 			if (ctrlc()) {
 			    putc ('\n');
-			    return 1;
+			    goto end;
 			}
 		    }
 		}

@@ -809,7 +819,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			    errs++;
 			    if (ctrlc()) {
 				putc ('\n');
-				return 1;
+				goto end;
 			    }
 			}
 		    }

@@ -851,7 +861,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			errs++;
 			if (ctrlc()) {
 			    putc ('\n');
-			    return 1;
+			    goto end;
 			}
 		    }

@@ -873,7 +883,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			errs++;
 			if (ctrlc()) {
 			    putc ('\n');
-			    return 1;
+			    goto end;
 			}
 		    }
 		    start[offset] = 0;

@@ -885,13 +895,14 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	for (;;) {
 		if (ctrlc()) {
 			putc ('\n');
-			return 1;
+			goto end;
 		}
 
 		if (iteration_limit && iterations > iteration_limit) {
 			printf("Tested %d iteration(s) with %lu errors.\n",
 				iterations-1, errs);
-			return errs != 0;
+			ret = errs != 0;
+			goto end;
 		}
 		++iterations;

@@ -918,7 +929,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 				errs++;
 				if (ctrlc()) {
 					putc ('\n');
-					return 1;
+					goto end;
 				}
 			}
 			val += incr;

@@ -939,7 +950,11 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		incr = -incr;
 	}
 #endif
-	return 0;	/* not reached */
+
+end:
+	if (dcache)
+		dcache_enable();
+	return ret;
 }

Mike Frysinger

2012-08-11 03:18 UTC #1 lore raw

PATCH 1/1 mtest: Disable dcache during test

Quoted 5 lines · On Friday 10 August 2012 15:16:15 Beno?t Th?baudeau wrote:

mtest is supposed to test many types of memory accesses in many different conditions. If dcache is enabled, memory accesses are likely bursts, and some memory accesses are simply skipped. Hence, dcache should be disabled during mtest operation so that what mtest actually tests is not masked by dcache.

if you want dcache disabled, then why don't you run dcache off first ? i think it's useful to be able to do both, and forcing it one way is wrong.

thus, NAK from me.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120810/0de99111/attachment.pgp>

Benoît Thébaudeau

author

2012-08-11 14:17 UTC #2 lore raw

PATCH 1/1 mtest: Disable dcache during test

Hi Mike,

Quoted 11 lines · Mike Frysinger

Quoted 9 lines · On Friday 10 August 2012 15:16:15 Beno?t Th?baudeau wrote:
mtest is supposed to test many types of memory accesses in many different
conditions. If dcache is enabled, memory accesses are likely bursts, and
some memory accesses are simply skipped. Hence, dcache should be disabled
during mtest operation so that what mtest actually tests is not masked by
dcache.

Quoted 7 lines

if you want dcache disabled, then why don't you run dcache off first ? i
think it's useful to be able to do both, and forcing it one way is wrong.
thus, NAK from me.
-mike

Because you will very likely trust mtest and forget about running dcache off first, so you may then be happy about falsely positive mtest results. Moreover, I can't find any sense or usefulness in running mtest with dcache enabled.

If there is really a need for running mtest with dcache enabled, an option (disabled by default) could be added to the command line to tell mtest not to disable dcache.

Best regards,
Beno?t

Mike Frysinger

2012-08-11 17:50 UTC #3 lore raw

PATCH 1/1 mtest: Disable dcache during test

Quoted 17 lines · On Saturday 11 August 2012 10:17:04 Beno?t Th?baudeau wrote:

Quoted 16 lines · Mike Frysinger
Quoted 9 lines · On Friday 10 August 2012 15:16:15 Beno?t Th?baudeau wrote:
mtest is supposed to test many types of memory accesses in many different
conditions. If dcache is enabled, memory accesses are likely bursts, and
some memory accesses are simply skipped. Hence, dcache should be disabled
during mtest operation so that what mtest actually tests is not masked by
dcache.
if you want dcache disabled, then why don't you run dcache off first ? i
think it's useful to be able to do both, and forcing it one way is wrong.

Quoted 4 lines

Quoted 1 line
thus, NAK from me.
Because you will very likely trust mtest and forget about running dcache off first, so you may then be happy about falsely positive mtest results.

there are a lot of commands in u-boot that people could "easily forget" to tweak and have it do the wrong thing. imo, mtest running in the active cache/memory settings is the *expected* behavior rather than having it silently reconfigure the system behind the back of the user.

Quoted 2 lines

Moreover, I can't find any sense or usefulness in running mtest with dcache enabled.

it can be useful to stress test the system with cache enabled. i recall during some early porting work that being able to test both caught bugs that we might have missed otherwise.

Quoted 3 lines

If there is really a need for running mtest with dcache enabled, an option (disabled by default) could be added to the command line to tell mtest not to disable dcache.

we already have such an option: dcache off; mtest; dcache on
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120811/f85ea029/attachment.pgp>

Benoît Thébaudeau

author

2012-08-11 18:49 UTC #4 lore raw

PATCH 1/1 mtest: Disable dcache during test

Hi Mike,

Quoted 27 lines · Mike Frysinger

Quoted 19 lines · On Saturday 11 August 2012 10:17:04 Beno?t Th?baudeau wrote:
Quoted 18 lines · Mike Frysinger
Quoted 10 lines · On Friday 10 August 2012 15:16:15 Beno?t Th?baudeau wrote:
mtest is supposed to test many types of memory accesses in many different
conditions. If dcache is enabled, memory accesses are likely bursts, and
some memory accesses are simply skipped. Hence, dcache should be
disabled
during mtest operation so that what mtest actually tests is not masked by
dcache.
if you want dcache disabled, then why don't you run dcache off first ? i
think it's useful to be able to do both, and forcing it one way is
wrong.
Quoted 6 lines
Quoted 1 line
thus, NAK from me.
Because you will very likely trust mtest and forget about running `dcache
off` first, so you may then be happy about falsely positive mtest results.

Quoted 7 lines

there are a lot of commands in u-boot that people could "easily forget" to
tweak and have it do the wrong thing. imo, mtest running in the active
cache/memory settings is the *expected* behavior rather than having it
silently reconfigure the system behind the back of the user.

IMO, mtest is supposed to test the underlying memory as it advertises, not the dcache system, so this is what people will expect from it, and this should be the default behavior. If dcache is needed in some cases, it should be an explicit choice, not the default one. People should not have to think about dcache being supported or enabled on their platform when running mtest. If dcache is added as a command line option to mtest, it will also appear in its help message, which will be a reminder, so there will be nothing made really silently. On the contrary, if dcache is left enabled by default, this will be completely unknown from the user, and there will not even be a reminder that disabling dcache manually should be considered.

Quoted 9 lines

Quoted 3 lines
Moreover, I can't find any sense or usefulness in running mtest with
dcache enabled.
it can be useful to stress test the system with cache enabled. i recall
during some early porting work that being able to test both caught bugs that
we might have missed otherwise.

OK, but this is not the main purpose of mtest. Here, you used it as a system stress test tool, not really as a memory stress test tool. So I agree that leaving dcache enabled should be an option, but not the default behavior.

Quoted 8 lines

Quoted 5 lines
If there is really a need for running mtest with dcache enabled, an option
(disabled by default) could be added to the command line to tell mtest not
to disable dcache.
we already have such an option: dcache off; mtest; dcache on -mike

See my comment above. IMO, dcache off should be the default for mtest.

If you really don't want that, the least should be to have a big warning printed both in mtest help message and when running mtest with dcache enabled.

Best regards,
Beno?t

Wolfgang Denk

2012-09-02 16:30 UTC #5 lore raw

PATCH 1/1 mtest: Disable dcache during test

In reply to Benoît Thébaudeau

Dear Beno?t Th?baudeau,

Quoted 2 lines · In message <[email protected]> you…

On 08/11/2012 05:18 AM, Mike Frysinger wrote:

...

Quoted 11 lines

Quoted 7 lines
if you want dcache disabled, then why don't you run dcache off first ? i
think it's useful to be able to do both, and forcing it one way is wrong.
thus, NAK from me.
-mike
Because you will very likely trust mtest and forget about running dcache off first, so you may then be happy about falsely positive mtest results. Moreover, I can't find any sense or usefulness in running mtest with dcache enabled.

I agree with Mike.

"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn

So NAK from me, too.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offence.
          -- Edsger W. Dijkstra, SIGPLAN Notices, Volume 17, Number 5

Benoît Thébaudeau

author

2012-09-03 14:25 UTC #6 lore raw

PATCH 1/1 mtest: Disable dcache during test

Dear Wolfgang Denk,

Quoted 24 lines · Wolfgang Denk

Dear Beno?t Th?baudeau,
In message <[email protected]> you
Quoted 2 lines · wrote:
On 08/11/2012 05:18 AM, Mike Frysinger wrote:
...
Quoted 9 lines
Quoted 8 lines
if you want dcache disabled, then why don't you run dcache off first ? i
think it's useful to be able to do both, and forcing it one way is
wrong.
thus, NAK from me.
-mike
Quoted 6 lines
Because you will very likely trust mtest and forget about running dcache off
first, so you may then be happy about falsely positive mtest results. Moreover,
I can't find any sense or usefulness in running mtest with dcache enabled.

Quoted 6 lines

I agree with Mike.
"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn
So NAK from me, too.

OK, but do you agree with the following that Mike and me agreed on after that?
http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
http://patchwork.ozlabs.org/patch/176909/

Best regards,
Beno?t

Albert ARIBAUD

2012-09-03 16:50 UTC #7 lore raw

PATCH 1/1 mtest: Disable dcache during test

Hi Beno?t,

Quoted 33 lines · Beno?t Th?baudeau

Dear Wolfgang Denk,
Quoted 30 lines · Wolfgang Denk
Dear Beno?t Th?baudeau,
In message <[email protected]> you
Quoted 2 lines · wrote:
On 08/11/2012 05:18 AM, Mike Frysinger wrote:
...
Quoted 9 lines
Quoted 8 lines
if you want dcache disabled, then why don't you run dcache off first ? i
think it's useful to be able to do both, and forcing it one way is
wrong.
thus, NAK from me.
-mike
Quoted 6 lines
Because you will very likely trust mtest and forget about running dcache off
first, so you may then be happy about falsely positive mtest results. Moreover,
I can't find any sense or usefulness in running mtest with dcache enabled.
I agree with Mike.
"UNIX was not designed to stop you from doing stupid things,
because that would also stop you from doing clever things." -
Doug Gwyn

Quoted 7 lines

Quoted 1 line
So NAK from me, too.
OK, but do you agree with the following that Mike and me agreed on after that?
http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
http://patchwork.ozlabs.org/patch/176909/

I did already reply to this, but since the agreement is brought back, I think I should re-state my opinion: such a warning line will most likely be overlooked, thus has little value. People using mtest should know that they must check/set dcache state before running mtest.

Quoted 2 lines

Best regards,
Beno?t

Amicalement,

Signature · 1 line

Albert.

Benoît Thébaudeau

author

2012-09-03 21:14 UTC #8 lore raw

PATCH 1/1 mtest: Disable dcache during test

Hi Albert,

Quoted 51 lines · Albert ARIBAUD

Hi Beno?t,
Quoted 38 lines · Beno?t Th?baudeau
Dear Wolfgang Denk,
Quoted 35 lines · Wolfgang Denk
Dear Beno?t Th?baudeau,
In message <[email protected]> you
Quoted 2 lines · wrote:
On 08/11/2012 05:18 AM, Mike Frysinger wrote:
...
Quoted 11 lines
Quoted 10 lines
if you want dcache disabled, then why don't you run dcache off
first ? i
think it's useful to be able to do both, and forcing it one way
is
wrong.
thus, NAK from me.
-mike
Quoted 8 lines
Because you will very likely trust mtest and forget about running
dcache off
first, so you may then be happy about falsely positive mtest results. Moreover,
I can't find any sense or usefulness in running mtest with dcache
enabled.
I agree with Mike.
"UNIX was not designed to stop you from doing stupid things, because that would also stop you from doing clever things."
      -
Doug Gwyn
Quoted 7 lines
Quoted 1 line
So NAK from me, too.
OK, but do you agree with the following that Mike and me agreed on after that?
http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
http://patchwork.ozlabs.org/patch/176909/

Quoted 6 lines

I did already reply to this, but since the agreement is brought back, I
think I should re-state my opinion: such a warning line will most likely
be overlooked, thus has little value. People using mtest should know that they must check/set dcache state before running mtest.

If this line is overlooked, it's the same as not having it. If it is not overlooked, it is useful both to detail the test conditions and as a reminder not to do stupid things. This line does not prevent users from doing any manual dcache check/enable/disable operation they want before running mtest. All in all, adding this line can only be beneficial.

Best regards,
Beno?t

Graeme Russ

2012-09-03 23:30 UTC #9 lore raw

PATCH 1/1 mtest: Disable dcache during test

Hi Beno?t,

Quoted 61 lines · Beno?t Th?baudeau

Hi Albert,
Quoted 51 lines · Albert ARIBAUD
Hi Beno?t,
Quoted 38 lines · Beno?t Th?baudeau
Dear Wolfgang Denk,
Quoted 35 lines · Wolfgang Denk
Dear Beno?t Th?baudeau,
In message <[email protected]> you
Quoted 2 lines · wrote:
On 08/11/2012 05:18 AM, Mike Frysinger wrote:
...
Quoted 11 lines
Quoted 10 lines
if you want dcache disabled, then why don't you run dcache off
first ? i
think it's useful to be able to do both, and forcing it one way
is
wrong.
thus, NAK from me.
-mike
Quoted 8 lines
Because you will very likely trust mtest and forget about running
dcache off
first, so you may then be happy about falsely positive mtest results. Moreover,
I can't find any sense or usefulness in running mtest with dcache
enabled.
I agree with Mike.
"UNIX was not designed to stop you from doing stupid things, because that would also stop you from doing clever things."
      -
Doug Gwyn
Quoted 7 lines
Quoted 1 line
So NAK from me, too.
OK, but do you agree with the following that Mike and me agreed on after that?
http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
http://patchwork.ozlabs.org/patch/176909/
Quoted 6 lines
I did already reply to this, but since the agreement is brought back, I
think I should re-state my opinion: such a warning line will most likely
be overlooked, thus has little value. People using mtest should know that they must check/set dcache state before running mtest.

Quoted 5 lines

If this line is overlooked, it's the same as not having it. If it is not overlooked, it is useful both to detail the test conditions and as a reminder not to do stupid things. This line does not prevent users from doing any manual dcache check/enable/disable operation they want before running mtest. All in all, adding this line can only be beneficial.

I agree - particularly when somebody else is looking at the output (on the ML for example) and notices it.

I'm also inclined to not 100% trust dcache operations (we all know that cache support is in a state of flux) so something like:

# dcache on
# mtest 0x80100000 0x90000000 0xaabb

If you get back:

Testing 0x80100000 ... 0x90000000 (dcache: off):

You know something is wrong with the dcache on implementation

Regards,

Graeme

Wolfgang Denk

2012-09-04 06:41 UTC #10 lore raw

PATCH 1/1 mtest: Disable dcache during test

Dear Graeme Russ,

Quoted 6 lines · In message <CALButCLVW0Vv9vc+MZ0wpau2xw7FPv+ZqpD7MZpxn=K_400HVA@mail.gmail.com>…

Quoted 6 lines
If this line is overlooked, it's the same as not having it. If it is not overlooked, it is useful both to detail the test conditions and as a reminder not to do stupid things. This line does not prevent users from doing any manual dcache check/enable/disable operation they want before running mtest. All in all, adding this line can only be beneficial.

I disagree. It add line noise, printing information that is available otherwise as well.

If you are interested in the state of the data cache support, just run the "dcache" command.

Quoted 5 lines

I'm also inclined to not 100% trust dcache operations (we all know that cache support is in a state of flux) so something like:
# dcache on
# mtest 0x80100000 0x90000000 0xaabb

...

Quoted 1 line

You know something is wrong with the dcache on implementation

Well, it appears awkward to me to use a completely unrelated command for such "testing". The same result should be available (much more straightforward) with:

	# dcache on
	# dcache

Best regards,

Wolfgang Denk

Signature · 7 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The word "fit", as I understand it, means "appropriate to a purpose",
and I would say the body of the Dean is supremely appropriate to  the
purpose of sitting around all day and eating big heavy meals.
                                 - Terry Pratchett, _Moving Pictures_

Graeme Russ

2012-09-04 06:43 UTC #11 lore raw

PATCH 1/1 mtest: Disable dcache during test

Hi Wolfgang,

Quoted 8 lines · Wolfgang Denk

Dear Graeme Russ,
Well, it appears awkward to me to use a completely unrelated command for such "testing". The same result should be available (much more straightforward) with:
        # dcache on
        # dcache

Good point :)

Regards,

Graeme

Wolfgang Denk

2012-09-04 07:23 UTC #12 lore raw

PATCH 1/1 mtest: Disable dcache during test

In reply to Benoît Thébaudeau

Dear Beno?t Th?baudeau,

Quoted 7 lines · In message <[email protected]> you…

Quoted 2 lines
So NAK from me, too.
OK, but do you agree with the following that Mike and me agreed on after that?
http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
http://patchwork.ozlabs.org/patch/176909/

No, I don't. If you anybody is interested in the state of the cache support, he can use the "dcache" (and/or "icache") command(s).

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Technology is dominated by those who manage what they do  not  under-
stand.