Subject: Icache coherency problems for R3400, DS5000/240

From: "Maciej W. Rozycki" <[email-masked]>
Date: Mon, 19 Jun 2000 11:31:27 +0200 (MET DST)

Ralf,

 Working on gdb I discovered a weird behaviour of my DS5000/240 -- under
unspecified circumstances there were spurious breakpoint traps happening
and some single-step breakpoints appeared persistent.  The following patch
fixes these problems, making gdb fully reliable.

 Besides obvious bugfixes, it introduces two significant changes.  First,
flush_icache_page() now performs what the name suggests, i.e. flushes the
instruction cache.  Without this change ptrace(PTRACE_POKE*, ...) calls
are unreliable.  Second, it changes the assumption of the icache line size
to a single word -- apparently, at least R3400 of DS5000/240 has an icache
with such a layout (DEC docs confirm it, indeed).  Without this change,
there are problems with breakpoints placed at addresses equal 4 modulo 8. 

 I vote for an immediate inclusion of these fixes.

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email-masked], PGP key available        +

diff -u --recursive --new-file linux-mips-2.3.99-pre8-20000523.macro/arch/mips/mm/r2300.c linux-mips-2.3.99-pre8-20000523/arch/mips/mm/r2300.c
--- linux-mips-2.3.99-pre8-20000523.macro/arch/mips/mm/r2300.c	Tue Mar 28 04:26:12 2000
+++ linux-mips-2.3.99-pre8-20000523/arch/mips/mm/r2300.c	Sun Jun 18 02:08:39 2000
@@ -151,15 +151,15 @@
 
 static void __init probe_dcache(void)
 {
-	dcache_size = r3k_cache_size(ST0_DE);
+	dcache_size = r3k_cache_size(ST0_ISC);
 	printk("Primary data cache %dkb, linesize 4 bytes\n",
 		dcache_size >> 10);
 }
 
 static void __init probe_icache(void)
 {
-	icache_size = r3k_cache_size(ST0_DE|ST0_CE);
-	printk("Primary instruction cache %dkb, linesize 8 bytes\n",
+	icache_size = r3k_cache_size(ST0_ISC|ST0_SWC);
+	printk("Primary instruction cache %dkb, linesize 4 bytes\n",
 		icache_size >> 10);
 }
 
@@ -174,43 +174,43 @@
 	save_and_cli(flags);
 
 	/* isolate cache space */
-	write_32bit_cp0_register(CP0_STATUS, (ST0_DE|ST0_CE|flags)&~ST0_IEC);
+	write_32bit_cp0_register(CP0_STATUS, (ST0_ISC|ST0_SWC|flags)&~ST0_IEC);
 
-	for (i = 0; i < size; i += 0x100) {
+	for (i = 0; i < size; i += 0x080) {
 		asm ( 	"sb\t$0,0x000(%0)\n\t"
+			"sb\t$0,0x004(%0)\n\t"
 			"sb\t$0,0x008(%0)\n\t"
+			"sb\t$0,0x00c(%0)\n\t"
 			"sb\t$0,0x010(%0)\n\t"
+			"sb\t$0,0x014(%0)\n\t"
 			"sb\t$0,0x018(%0)\n\t"
-			"sb\t$0,0x020(%0)\n\t"
+			"sb\t$0,0x01c(%0)\n\t"
+		 	"sb\t$0,0x020(%0)\n\t"
+			"sb\t$0,0x024(%0)\n\t"
 			"sb\t$0,0x028(%0)\n\t"
+			"sb\t$0,0x02c(%0)\n\t"
 			"sb\t$0,0x030(%0)\n\t"
+			"sb\t$0,0x034(%0)\n\t"
 			"sb\t$0,0x038(%0)\n\t"
-		 	"sb\t$0,0x040(%0)\n\t"
+			"sb\t$0,0x03c(%0)\n\t"
+			"sb\t$0,0x040(%0)\n\t"
+			"sb\t$0,0x044(%0)\n\t"
 			"sb\t$0,0x048(%0)\n\t"
+			"sb\t$0,0x04c(%0)\n\t"
 			"sb\t$0,0x050(%0)\n\t"
+			"sb\t$0,0x054(%0)\n\t"
 			"sb\t$0,0x058(%0)\n\t"
-			"sb\t$0,0x060(%0)\n\t"
+			"sb\t$0,0x05c(%0)\n\t"
+		 	"sb\t$0,0x060(%0)\n\t"
+			"sb\t$0,0x064(%0)\n\t"
 			"sb\t$0,0x068(%0)\n\t"
+			"sb\t$0,0x06c(%0)\n\t"
 			"sb\t$0,0x070(%0)\n\t"
+			"sb\t$0,0x074(%0)\n\t"
 			"sb\t$0,0x078(%0)\n\t"
-			"sb\t$0,0x080(%0)\n\t"
-			"sb\t$0,0x088(%0)\n\t"
-			"sb\t$0,0x090(%0)\n\t"
-			"sb\t$0,0x098(%0)\n\t"
-			"sb\t$0,0x0a0(%0)\n\t"
-			"sb\t$0,0x0a8(%0)\n\t"
-			"sb\t$0,0x0b0(%0)\n\t"
-			"sb\t$0,0x0b8(%0)\n\t"
-		 	"sb\t$0,0x0c0(%0)\n\t"
-			"sb\t$0,0x0c8(%0)\n\t"
-			"sb\t$0,0x0d0(%0)\n\t"
-			"sb\t$0,0x0d8(%0)\n\t"
-			"sb\t$0,0x0e0(%0)\n\t"
-			"sb\t$0,0x0e8(%0)\n\t"
-			"sb\t$0,0x0f0(%0)\n\t"
-			"sb\t$0,0x0f8(%0)\n\t"
+			"sb\t$0,0x07c(%0)\n\t"
 			: : "r" (p) );
-		p += 0x100;
+		p += 0x080;
 	}
 
 	restore_flags(flags);
@@ -221,13 +221,13 @@
 	unsigned long i, flags;
 	volatile unsigned char *p = (char *)start;
 
-	if (size > icache_size)
-		size = icache_size;
+	if (size > dcache_size)
+		size = dcache_size;
 
 	save_and_cli(flags);
 
 	/* isolate cache space */
-	write_32bit_cp0_register(CP0_STATUS, (ST0_DE|flags)&~ST0_IEC);
+	write_32bit_cp0_register(CP0_STATUS, (ST0_ISC|flags)&~ST0_IEC);
 
 	for (i = 0; i < size; i += 0x080) {
 		asm ( 	"sb\t$0,0x000(%0)\n\t"
@@ -376,7 +376,7 @@
 
 	save_and_cli(flags);
 
-	write_32bit_cp0_register(CP0_STATUS, (ST0_DE|ST0_CE|flags)&~ST0_IEC);
+	write_32bit_cp0_register(CP0_STATUS, (ST0_ISC|ST0_SWC|flags)&~ST0_IEC);
 
 	asm ( 	"sb\t$0,0x000(%0)\n\t"
 		"sb\t$0,0x008(%0)\n\t"
diff -u --recursive --new-file linux-mips-2.3.99-pre8-20000523.macro/include/asm-mips/mipsregs.h linux-mips-2.3.99-pre8-20000523/include/asm-mips/mipsregs.h
--- linux-mips-2.3.99-pre8-20000523.macro/include/asm-mips/mipsregs.h	Sat May 27 01:49:02 2000
+++ linux-mips-2.3.99-pre8-20000523/include/asm-mips/mipsregs.h	Sun Jun 18 02:18:51 2000
@@ -234,6 +234,8 @@
 #define ST0_UX			0x00000020
 #define ST0_SX			0x00000040
 #define ST0_KX 			0x00000080
+#define ST0_DE			0x00010000
+#define ST0_CE			0x00020000
 
 /*
  * Bitfields in the R[23]000 cp0 status register.
@@ -245,6 +247,8 @@
 #define ST0_IEO			0x00000010
 #define ST0_KUO			0x00000020
 /* bits 6 & 7 are reserved on R[23]000 */
+#define ST0_ISC			0x00010000
+#define ST0_SWC			0x00020000
 
 /*
  * Bits specific to the R4640/R4650
@@ -273,8 +277,6 @@
 #define  STATUSF_IP6		(1   << 14)
 #define  STATUSB_IP7		15
 #define  STATUSF_IP7		(1   << 15)
-#define ST0_DE			0x00010000
-#define ST0_CE			0x00020000
 #define ST0_CH			0x00040000
 #define ST0_SR			0x00100000
 #define ST0_BEV			0x00400000
diff -u --recursive --new-file linux-mips-2.3.99-pre8-20000523.macro/include/asm-mips/pgtable.h linux-mips-2.3.99-pre8-20000523/include/asm-mips/pgtable.h
--- linux-mips-2.3.99-pre8-20000523.macro/include/asm-mips/pgtable.h	Tue Jun  6 23:21:21 2000
+++ linux-mips-2.3.99-pre8-20000523/include/asm-mips/pgtable.h	Sat Jun 17 19:35:42 2000
@@ -43,7 +43,7 @@
 #define flush_page_to_ram(page)		_flush_page_to_ram(page)
 
 #define flush_icache_range(start, end)	flush_cache_all()
-#define flush_icache_page(start,page)	do { } while(0)
+#define flush_icache_page(start,page)	flush_cache_all()

 /*

---

From: Ralf Baechle <[email-masked]>
Date: Tue, 20 Jun 2000 00:04:55 +0200

>  I vote for an immediate inclusion of these fixes.

Looks good unless one of the R3000 gurus objects, so I'll add this right
after my ac21 commit finishes.

  Ralf

---

From: Dominic Sweetman <[email-masked]>
Date: Tue, 20 Jun 2000 00:46:11 +0100 (BST)

> > ... Second, it changes the assumption of the icache line size to a
> > single word -- apparently, at least R3400 of DS5000/240 has an
> > icache with such a layout (DEC docs confirm it, indeed).

Yes, the original R3000 chip could be wired to produce the appearance
of multi-word lines in its I-cache, and some derivative CPUs were built
that way.  Four was popular - I don't think anyone did 8.

> > Besides obvious bugfixes, it introduces two significant changes.
> > First, flush_icache_page() now performs what the name suggests,
> > i.e. flushes the instruction cache.

And while I'm here, I'll continue my lonely campaign.  I suggest you
don't say "flush" because nobody knows whether it means invalidate,
write-back, or both[1].  Instead, say "invalidate", "writeback", or
"both".  Even if it means changing St Linus' function names...

[1] well in this case we do, because this is an I-cache and R3000s
    only had write-through caches anyway.  But you weren't going to
    stop there, were you?

Dominic
Algorithmics Ltd
[email-masked]

---

From: Ralf Baechle <[email-masked]>
Date: Tue, 20 Jun 2000 01:52:24 +0200

> "both".  Even if it means changing St Linus' function names...

Sigh, I tried exactly this and got fire from the 68k fraction because
they considered my naming ambigous.

  Ralf

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Tue, 20 Jun 2000 14:50:05 +0200 (MET DST)

> that way.  Four was popular - I don't think anyone did 8.

 DEC docs claim the cache subsystem of KN03 is configured with single-word
lines for the icache, but upon an icache fill the MB ASIC fills four
lines.

> "both".  Even if it means changing St Linus' function names...

 OK, by "flush" I always meant "invalidate" (see also ia32's invd and
wbinvd instructions -- the first one causes an external flush cycle and
the other one an external writeback cycle and then a flush cycle -- that's
Intel's original naming).  On the other hand, "invalidate" is surely not
more ambiguous than "flush", so feel free to continue your campaign... 
;-)

---

From: Dominic Sweetman <[email-masked]>
Date: Tue, 20 Jun 2000 14:45:46 +0100 (BST)

> lines for the icache, but upon an icache fill the MB ASIC fills four

Ah, I'm suffering from muddy memory syndrome.  

You're quite correct: on the R3000 cache every word had a tag, but an
I-cache refill could be setup to fill several of them at once -
implicitly storing the same tag in each position.  I believe it was in
theory possible to gently remove the two lowest tag addresses, use a
smaller tag memory, and have a genuine four-data per tag I-cache.  It
would have behaved very strangely while swapped for
diagnostics/maintenance.

> wbinvd instructions -- the first one causes an external flush
> cycle...

I should have realised that Linux ab-usages usually refer to x86
technology.  Thanks for that.