[PATCH 0/8] 8xx: Misc fixes for buggy insn

[PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Here is the latest(last?) round of this series. I
hope I got everything right now.
Scott and Rex, please test and send ACK/NACK.

  Jocke

Joakim Tjernlund (8):
  8xx: invalidate non present TLBs
  8xx: Update TLB asm so it behaves as linux mm expects.
  8xx: Tag DAR with 0x00f0 to catch buggy instructions.
  8xx: Fixup DAR from buggy dcbX instructions.
  8xx: Add missing Guarded setting in DTLB Error.
  8xx: Restore _PAGE_WRITETHRU
  8xx: start using dcbX instructions in various copy routines
  8xx: Remove DIRTY pte handling in DTLB Error.

 arch/powerpc/include/asm/pte-8xx.h |   14 +-
 arch/powerpc/kernel/head_8xx.S     |  308 ++++++++++++++++++++++--------------
 arch/powerpc/kernel/misc_32.S      |   18 --
 arch/powerpc/lib/copy_32.S         |   24 ---
 arch/powerpc/mm/fault.c            |    8 +-
 5 files changed, 205 insertions(+), 167 deletions(-)

[PATCH 1/8] 8xx: invalidate non present TLBs

[Joakim Tjernlund]

8xx sometimes need to load a invalid/non-present TLBs in
it DTLB asm handler.
These must be invalidated separaly as linux mm don't.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/mm/fault.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index e7dae82..26fb6b9 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -40,7 +40,7 @@
 #include <asm/uaccess.h>
 #include <asm/tlbflush.h>
 #include <asm/siginfo.h>
-
+#include <mm/mmu_decl.h>
 
 #ifdef CONFIG_KPROBES
 static inline int notify_page_fault(struct pt_regs *regs)
@@ -246,6 +246,12 @@ good_area:
 		goto bad_area;
 #endif /* CONFIG_6xx */
 #if defined(CONFIG_8xx)
+	/* 8xx sometimes need to load a invalid/non-present TLBs.
+	 * These must be invalidated separately as linux mm don't.
+	 */
+	if (error_code & 0x40000000) /* no translation? */
+		_tlbil_va(address, 0, 0, 0);
+
         /* The MPC8xx seems to always set 0x80000000, which is
          * "undefined".  Of those that can be set, this is the only
          * one which seems bad.
-- 
1.6.4.4

[PATCH 2/8] 8xx: Update TLB asm so it behaves as linux mm expects.

[Joakim Tjernlund]

Update the TLB asm to make proper use of _PAGE_DIRY and _PAGE_ACCESSED.
Get rid of _PAGE_HWWRITE too.
Pros:
 - I/D TLB Miss never needs to write to the linux pte.
 - _PAGE_ACCESSED is only set on TLB Error fixing accounting
 - _PAGE_DIRTY is mapped to 0x100, the changed bit, and is set directly
    when a page has been made dirty.
 - Proper RO/RW mapping of user space.
 - Free up 2 SW TLB bits in the linux pte(add back _PAGE_WRITETHRU ?)
 - kernel RO/user NA support.
Cons:
 - A few more instructions in the TLB Miss routines.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/include/asm/pte-8xx.h |   13 ++---
 arch/powerpc/kernel/head_8xx.S     |   99 ++++++++++++++++++-----------------
 2 files changed, 57 insertions(+), 55 deletions(-)

diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
index dd5ea95..68ba861 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -33,21 +33,20 @@
 #define _PAGE_NO_CACHE	0x0002	/* I: cache inhibit */
 #define _PAGE_SHARED	0x0004	/* No ASID (context) compare */
 #define _PAGE_SPECIAL	0x0008	/* SW entry, forced to 0 by the TLB miss */
+#define _PAGE_DIRTY	0x0100	/* C: page changed */
 
-/* These five software bits must be masked out when the entry is loaded
- * into the TLB.
+/* These 3 software bits must be masked out when the entry is loaded
+ * into the TLB, 2 SW bits left.
  */
 #define _PAGE_GUARDED	0x0010	/* software: guarded access */
-#define _PAGE_DIRTY	0x0020	/* software: page changed */
-#define _PAGE_RW	0x0040	/* software: user write access allowed */
-#define _PAGE_ACCESSED	0x0080	/* software: page referenced */
+#define _PAGE_ACCESSED	0x0020	/* software: page referenced */
 
 /* Setting any bits in the nibble with the follow two controls will
  * require a TLB exception handler change.  It is assumed unused bits
  * are always zero.
  */
-#define _PAGE_HWWRITE	0x0100	/* h/w write enable: never set in Linux PTE */
-#define _PAGE_USER	0x0800	/* One of the PP bits, the other is USER&~RW */
+#define _PAGE_RW	0x0400	/* lsb PP bits, inverted in HW */
+#define _PAGE_USER	0x0800	/* msb PP bits */
 
 #define _PMD_PRESENT	0x0001
 #define _PMD_BAD	0x0ff0
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 6ded19d..97bd523 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -333,26 +333,20 @@ InstructionTLBMiss:
 	mfspr	r11, SPRN_MD_TWC	/* ....and get the pte address */
 	lwz	r10, 0(r11)	/* Get the pte */
 
-#ifdef CONFIG_SWAP
-	/* do not set the _PAGE_ACCESSED bit of a non-present page */
-	andi.	r11, r10, _PAGE_PRESENT
-	beq	4f
-	ori	r10, r10, _PAGE_ACCESSED
-	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
-	stw	r10, 0(r11)
-4:
-#else
-	ori	r10, r10, _PAGE_ACCESSED
-	stw	r10, 0(r11)
-#endif
+	andi.	r11, r10, _PAGE_ACCESSED | _PAGE_PRESENT
+	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
+	bne-	cr0, 2f
+
+	/* Clear PP lsb, 0x400 */
+	rlwinm 	r10, r10, 0, 22, 20
 
 	/* The Linux PTE won't go exactly into the MMU TLB.
-	 * Software indicator bits 21, 22 and 28 must be clear.
+	 * Software indicator bits 22 and 28 must be clear.
 	 * Software indicator bits 24, 25, 26, and 27 must be
 	 * set.  All other Linux PTE bits control the behavior
 	 * of the MMU.
 	 */
-2:	li	r11, 0x00f0
+	li	r11, 0x00f0
 	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
 	DO_8xx_CPU6(0x2d80, r3)
 	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */
@@ -365,6 +359,22 @@ InstructionTLBMiss:
 	lwz	r3, 8(r0)
 #endif
 	rfi
+2:
+	mfspr	r11, SPRN_SRR1
+	/* clear all error bits as TLB Miss
+	 * sets a few unconditionally
+	*/
+	rlwinm	r11, r11, 0, 0xffff
+	mtspr	SPRN_SRR1, r11
+
+	mfspr	r10, SPRN_M_TW	/* Restore registers */
+	lwz	r11, 0(r0)
+	mtcr	r11
+	lwz	r11, 4(r0)
+#ifdef CONFIG_8xx_CPU6
+	lwz	r3, 8(r0)
+#endif
+	b	InstructionAccess
 
 	. = 0x1200
 DataStoreTLBMiss:
@@ -409,21 +419,27 @@ DataStoreTLBMiss:
 	DO_8xx_CPU6(0x3b80, r3)
 	mtspr	SPRN_MD_TWC, r11
 
-#ifdef CONFIG_SWAP
-	/* do not set the _PAGE_ACCESSED bit of a non-present page */
-	andi.	r11, r10, _PAGE_PRESENT
-	beq	4f
-	ori	r10, r10, _PAGE_ACCESSED
-4:
-	/* and update pte in table */
-#else
-	ori	r10, r10, _PAGE_ACCESSED
-#endif
-	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
-	stw	r10, 0(r11)
+	/* Both _PAGE_ACCESSED and _PAGE_PRESENT has to be set.
+	 * We also need to know if the insn is a load/store, so:
+	 * Clear _PAGE_PRESENT and load that which will
+	 * trap into DTLB Error with store bit set accordinly.
+	 */
+	/* PRESENT=0x1, ACCESSED=0x20
+	 * r11 = ((r10 & PRESENT) & ((r10 & ACCESSED) >> 5));
+	 * r10 = (r10 & ~PRESENT) | r11;
+	 */
+	rlwinm	r11, r10, 32-5, 31, 31
+	and	r11, r11, r10
+	rlwimi	r10, r11, 0, 31, 31
+
+	/* Honour kernel RO, User NA */
+	andi.	r11, r10, _PAGE_USER | _PAGE_RW
+	bne-	cr0, 5f
+	ori	r10,r10, 0x200 /* Extended encoding, bit 22 */
+5:	xori	r10, r10, _PAGE_RW  /* invert RW bit */
 
 	/* The Linux PTE won't go exactly into the MMU TLB.
-	 * Software indicator bits 21, 22 and 28 must be clear.
+	 * Software indicator bits 22 and 28 must be clear.
 	 * Software indicator bits 24, 25, 26, and 27 must be
 	 * set.  All other Linux PTE bits control the behavior
 	 * of the MMU.
@@ -469,11 +485,12 @@ DataTLBError:
 	stw	r10, 0(r0)
 	stw	r11, 4(r0)
 
-	/* First, make sure this was a store operation.
+	mfspr	r11, SPRN_DSISR
+	andis.	r11, r11, 0x4800	/* !translation or protection */
+	bne	2f	/* branch if either is set */
+	/* Only Change bit left now, do it here as it is faster
+	 * than trapping to the C fault handler.
 	*/
-	mfspr	r10, SPRN_DSISR
-	andis.	r11, r10, 0x0200	/* If set, indicates store op */
-	beq	2f
 
 	/* The EA of a data TLB miss is automatically stored in the MD_EPN
 	 * register.  The EA of a data TLB error is automatically stored in
@@ -522,26 +539,12 @@ DataTLBError:
 	mfspr	r11, SPRN_MD_TWC		/* ....and get the pte address */
 	lwz	r10, 0(r11)		/* Get the pte */
 
-	andi.	r11, r10, _PAGE_RW	/* Is it writeable? */
-	beq	2f			/* Bail out if not */
-
-	/* Update 'changed', among others.
-	*/
-#ifdef CONFIG_SWAP
-	ori	r10, r10, _PAGE_DIRTY|_PAGE_HWWRITE
-	/* do not set the _PAGE_ACCESSED bit of a non-present page */
-	andi.	r11, r10, _PAGE_PRESENT
-	beq	4f
-	ori	r10, r10, _PAGE_ACCESSED
-4:
-#else
-	ori	r10, r10, _PAGE_DIRTY|_PAGE_ACCESSED|_PAGE_HWWRITE
-#endif
-	mfspr	r11, SPRN_MD_TWC		/* Get pte address again */
+	ori	r10, r10, _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_HWWRITE
 	stw	r10, 0(r11)		/* and update pte in table */
+	xori	r10, r10, _PAGE_RW	/* RW bit is inverted */
 
 	/* The Linux PTE won't go exactly into the MMU TLB.
-	 * Software indicator bits 21, 22 and 28 must be clear.
+	 * Software indicator bits 22 and 28 must be clear.
 	 * Software indicator bits 24, 25, 26, and 27 must be
 	 * set.  All other Linux PTE bits control the behavior
 	 * of the MMU.
-- 
1.6.4.4

[PATCH 3/8] 8xx: Tag DAR with 0x00f0 to catch buggy instructions.

[Joakim Tjernlund]

dcbz, dcbf, dcbi, dcbst and icbi do not set DAR when they
cause a DTLB Error. Dectect this by tagging DAR with 0x00f0
at every exception exit that modifies DAR.
Test for DAR=0x00f0 in DataTLBError and bail
to handle_page_fault().

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/kernel/head_8xx.S |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 97bd523..a9f1ace 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -206,6 +206,8 @@ MachineCheck:
 	EXCEPTION_PROLOG
 	mfspr r4,SPRN_DAR
 	stw r4,_DAR(r11)
+	li r5,0x00f0
+	mtspr SPRN_DAR,r5	/* Tag DAR, to be used in DTLB Error */
 	mfspr r5,SPRN_DSISR
 	stw r5,_DSISR(r11)
 	addi r3,r1,STACK_FRAME_OVERHEAD
@@ -222,6 +224,8 @@ DataAccess:
 	stw	r10,_DSISR(r11)
 	mr	r5,r10
 	mfspr	r4,SPRN_DAR
+	li	r10,0x00f0
+	mtspr	SPRN_DAR,r10	/* Tag DAR, to be used in DTLB Error */
 	EXC_XFER_EE_LITE(0x300, handle_page_fault)
 
 /* Instruction access exception.
@@ -244,6 +248,8 @@ Alignment:
 	EXCEPTION_PROLOG
 	mfspr	r4,SPRN_DAR
 	stw	r4,_DAR(r11)
+	li	r5,0x00f0
+	mtspr	SPRN_DAR,r5	/* Tag DAR, to be used in DTLB Error */
 	mfspr	r5,SPRN_DSISR
 	stw	r5,_DSISR(r11)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -445,6 +451,7 @@ DataStoreTLBMiss:
 	 * of the MMU.
 	 */
 2:	li	r11, 0x00f0
+	mtspr	SPRN_DAR,r11	/* Tag DAR */
 	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
 	DO_8xx_CPU6(0x3d80, r3)
 	mtspr	SPRN_MD_RPN, r10	/* Update TLB entry */
@@ -485,6 +492,10 @@ DataTLBError:
 	stw	r10, 0(r0)
 	stw	r11, 4(r0)
 
+	mfspr	r10, SPRN_DAR
+	cmpwi	cr0, r10, 0x00f0
+	beq-	2f	/* must be a buggy dcbX, icbi insn. */
+
 	mfspr	r11, SPRN_DSISR
 	andis.	r11, r11, 0x4800	/* !translation or protection */
 	bne	2f	/* branch if either is set */
@@ -508,7 +519,8 @@ DataTLBError:
 	 * are initialized in mapin_ram().  This will avoid the problem,
 	 * assuming we only use the dcbi instruction on kernel addresses.
 	 */
-	mfspr	r10, SPRN_DAR
+
+	/* DAR is in r10 already */
 	rlwinm	r11, r10, 0, 0, 19
 	ori	r11, r11, MD_EVALID
 	mfspr	r10, SPRN_M_CASID
@@ -550,6 +562,7 @@ DataTLBError:
 	 * of the MMU.
 	 */
 	li	r11, 0x00f0
+	mtspr	SPRN_DAR,r11	/* Tag DAR */
 	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
 	DO_8xx_CPU6(0x3d80, r3)
 	mtspr	SPRN_MD_RPN, r10	/* Update TLB entry */
-- 
1.6.4.4

[PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.

[Joakim Tjernlund]

This is an assembler version to fixup DAR not being set
by dcbX, icbi instructions. There are two versions, one
uses selfmodifing code, the other uses a
jump table but is much bigger(default).

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/kernel/head_8xx.S |  147 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index a9f1ace..1d8e4e3 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -494,11 +494,16 @@ DataTLBError:
 
 	mfspr	r10, SPRN_DAR
 	cmpwi	cr0, r10, 0x00f0
-	beq-	2f	/* must be a buggy dcbX, icbi insn. */
-
+	beq-	FixupDAR	/* must be a buggy dcbX, icbi insn. */
+DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR */
 	mfspr	r11, SPRN_DSISR
-	andis.	r11, r11, 0x4800	/* !translation or protection */
-	bne	2f	/* branch if either is set */
+	/* As the DAR fixup may clear store we may have all 3 states zero.
+	 * Make sure only 0x0200(store) falls down into DIRTY handling
+	 */
+	andis.	r11, r11, 0x4a00	/* !translation, protection or store */
+	srwi	r11, r11, 16
+	cmpwi	cr0, r11, 0x0200	/* just store ? */
+	bne	2f
 	/* Only Change bit left now, do it here as it is faster
 	 * than trapping to the C fault handler.
 	*/
@@ -604,6 +609,140 @@ DataTLBError:
 
 	. = 0x2000
 
+/* This is the procedure to calculate the data EA for buggy dcbx,dcbi instructions
+ * by decoding the registers used by the dcbx instruction and adding them.
+ * DAR is set to the calculated address and r10 also holds the EA on exit.
+ */
+ /* define if you don't want to use self modifying code */
+#define NO_SELF_MODIFYING_CODE
+FixupDAR:/* Entry point for dcbx workaround. */
+	/* fetch instruction from memory. */
+	mfspr	r10, SPRN_SRR0
+	DO_8xx_CPU6(0x3780, r3)
+	mtspr	SPRN_MD_EPN, r10
+	mfspr	r11, SPRN_M_TWB	/* Get level 1 table entry address */
+	cmplwi	cr0, r11, 0x0800
+	blt-	3f		/* Branch if user space */
+	lis	r11, (swapper_pg_dir-PAGE_OFFSET)@h
+	ori	r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
+	rlwimi	r11, r10, 22, 0xffc
+3:	lwz	r11, 0(r11)	/* Get the level 1 entry */
+	DO_8xx_CPU6(0x3b80, r3)
+	mtspr	SPRN_MD_TWC, r11	/* Load pte table base address */
+	mfspr	r11, SPRN_MD_TWC	/* ....and get the pte address */
+	lwz	r11, 0(r11)	/* Get the pte */
+	/* concat physical page address(r11) and page offset(r10) */
+	rlwimi	r11, r10, 0, 20, 31
+	lwz	r11,0(r11)
+/* Check if it really is a dcbx instruction. */
+/* dcbt and dcbtst does not generate DTLB Misses/Errors,
+ * no need to include them here */
+	srwi	r10, r11, 26	/* check if major OP code is 31 */
+	cmpwi	cr0, r10, 31
+	bne-	141f
+	rlwinm	r10, r11, 0, 21, 30
+	cmpwi	cr0, r10, 2028	/* Is dcbz? */
+	beq+	142f
+	cmpwi	cr0, r10, 940	/* Is dcbi? */
+	beq+	142f
+	cmpwi	cr0, r10, 108	/* Is dcbst? */
+	beq+	144f		/* Fix up store bit! */
+	cmpwi	cr0, r10, 172	/* Is dcbf? */
+	beq+	142f
+	cmpwi	cr0, r10, 1964	/* Is icbi? */
+	beq+	142f
+141:	mfspr	r10, SPRN_DAR	/* r10 must hold DAR at exit */
+	b	DARFixed	/* Nope, go back to normal TLB processing */
+
+144:	mfspr	r10, SPRN_DSISR
+	rlwinm	r10, r10,0,7,5	/* Clear store bit for buggy dcbst insn */
+	mtspr	SPRN_DSISR, r10
+142:	/* continue, it was a dcbx, dcbi instruction. */
+#ifdef CONFIG_8xx_CPU6
+	lwz	r3, 8(r0)	/* restore r3 from memory */
+#endif
+#ifndef NO_SELF_MODIFYING_CODE
+	andis.	r10,r11,0x1f	/* test if reg RA is r0 */
+	li	r10,modified_instr@l
+	dcbtst	r0,r10		/* touch for store */
+	rlwinm	r11,r11,0,0,20	/* Zero lower 10 bits */
+	oris	r11,r11,640	/* Transform instr. to a "add r10,RA,RB" */
+	ori	r11,r11,532
+	stw	r11,0(r10)	/* store add/and instruction */
+	dcbf	0,r10		/* flush new instr. to memory. */
+	icbi	0,r10		/* invalidate instr. cache line */
+	lwz	r11, 4(r0)	/* restore r11 from memory */
+	mfspr	r10, SPRN_M_TW	/* restore r10 from M_TW */
+	isync			/* Wait until new instr is loaded from memory */
+modified_instr:
+	.space	4		/* this is where the add/and instr. is stored */
+	bne+	143f
+	subf	r10,r0,r10	/* r10=r10-r0, only if reg RA is r0 */
+143:	mtdar	r10		/* store faulting EA in DAR */
+	b	DARFixed	/* Go back to normal TLB handling */
+#else
+	mfctr	r10
+	mtdar	r10			/* save ctr reg in DAR */
+	rlwinm	r10, r11, 24, 24, 28	/* offset into jump table for reg RB */
+	addi	r10, r10, 150f@l	/* add start of table */
+	mtctr	r10			/* load ctr with jump address */
+	xor	r10, r10, r10		/* sum starts at zero */
+	bctr				/* jump into table */
+150:
+	add	r10, r10, r0	;b	151f
+	add	r10, r10, r1	;b	151f
+	add	r10, r10, r2	;b	151f
+	add	r10, r10, r3	;b	151f
+	add	r10, r10, r4	;b	151f
+	add	r10, r10, r5	;b	151f
+	add	r10, r10, r6	;b	151f
+	add	r10, r10, r7	;b	151f
+	add	r10, r10, r8	;b	151f
+	add	r10, r10, r9	;b	151f
+	mtctr	r11	;b	154f	/* r10 needs special handling */
+	mtctr	r11	;b	153f	/* r11 needs special handling */
+	add	r10, r10, r12	;b	151f
+	add	r10, r10, r13	;b	151f
+	add	r10, r10, r14	;b	151f
+	add	r10, r10, r15	;b	151f
+	add	r10, r10, r16	;b	151f
+	add	r10, r10, r17	;b	151f
+	add	r10, r10, r18	;b	151f
+	add	r10, r10, r19	;b	151f
+	add	r10, r10, r20	;b	151f
+	add	r10, r10, r21	;b	151f
+	add	r10, r10, r22	;b	151f
+	add	r10, r10, r23	;b	151f
+	add	r10, r10, r24	;b	151f
+	add	r10, r10, r25	;b	151f
+	add	r10, r10, r25	;b	151f
+	add	r10, r10, r27	;b	151f
+	add	r10, r10, r28	;b	151f
+	add	r10, r10, r29	;b	151f
+	add	r10, r10, r30	;b	151f
+	add	r10, r10, r31
+151:
+	rlwinm. r11,r11,19,24,28	/* offset into jump table for reg RA */
+	beq	152f			/* if reg RA is zero, don't add it */ 
+	addi	r11, r11, 150b@l	/* add start of table */
+	mtctr	r11			/* load ctr with jump address */
+	rlwinm	r11,r11,0,16,10		/* make sure we don't execute this more than once */
+	bctr				/* jump into table */
+152:
+	mfdar	r11
+	mtctr	r11			/* restore ctr reg from DAR */
+	mtdar	r10			/* save fault EA to DAR */
+	b	DARFixed		/* Go back to normal TLB handling */
+
+	/* special handling for r10,r11 since these are modified already */
+153:	lwz	r11, 4(r0)	/* load r11 from memory */
+	b	155f
+154:	mfspr	r11, SPRN_M_TW	/* load r10 from M_TW */
+155:	add	r10, r10, r11	/* add it */
+	mfctr	r11		/* restore r11 */
+	b	151b
+#endif
+
 	.globl	giveup_fpu
 giveup_fpu:
 	blr
-- 
1.6.4.4

[PATCH 5/8] 8xx: Add missing Guarded setting in DTLB Error.

[Joakim Tjernlund]

only DTLB Miss did set this bit, DTLB Error needs too otherwise
the setting is lost when the page becomes dirty.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/kernel/head_8xx.S |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 1d8e4e3..18e9901 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -552,9 +552,16 @@ DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR
 	 */
 	ori	r11, r11, 1		/* Set valid bit in physical L2 page */
 	DO_8xx_CPU6(0x3b80, r3)
-	mtspr	SPRN_MD_TWC, r11		/* Load pte table base address */
-	mfspr	r11, SPRN_MD_TWC		/* ....and get the pte address */
-	lwz	r10, 0(r11)		/* Get the pte */
+	mtspr	SPRN_MD_TWC, r11	/* Load pte table base address */
+	mfspr	r10, SPRN_MD_TWC	/* ....and get the pte address */
+	lwz	r10, 0(r10)		/* Get the pte */
+	/* Insert the Guarded flag into the TWC from the Linux PTE.
+	 * It is bit 27 of both the Linux PTE and the TWC
+	 */
+	rlwimi	r11, r10, 0, 27, 27
+	DO_8xx_CPU6(0x3b80, r3)
+	mtspr	SPRN_MD_TWC, r11
+	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
 
 	ori	r10, r10, _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_HWWRITE
 	stw	r10, 0(r11)		/* and update pte in table */
-- 
1.6.4.4

[PATCH 6/8] 8xx: Restore _PAGE_WRITETHRU

[Joakim Tjernlund]

8xx has not had WRITETHRU due to lack of bits in the pte.
After the recent rewrite of the 8xx TLB code, there are
two bits left. Use one of them to WRITETHRU.

Perhaps use the last SW bit to PAGE_SPECIAL or PAGE_FILE?

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/include/asm/pte-8xx.h |    5 +++--
 arch/powerpc/kernel/head_8xx.S     |    8 ++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
index 68ba861..d44826e 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -35,11 +35,12 @@
 #define _PAGE_SPECIAL	0x0008	/* SW entry, forced to 0 by the TLB miss */
 #define _PAGE_DIRTY	0x0100	/* C: page changed */
 
-/* These 3 software bits must be masked out when the entry is loaded
- * into the TLB, 2 SW bits left.
+/* These 4 software bits must be masked out when the entry is loaded
+ * into the TLB, 1 SW bit left(0x0080).
  */
 #define _PAGE_GUARDED	0x0010	/* software: guarded access */
 #define _PAGE_ACCESSED	0x0020	/* software: page referenced */
+#define _PAGE_WRITETHRU	0x0040	/* software: caching is write through */
 
 /* Setting any bits in the nibble with the follow two controls will
  * require a TLB exception handler change.  It is assumed unused bits
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 18e9901..65e0526 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -422,6 +422,10 @@ DataStoreTLBMiss:
 	 * above.
 	 */
 	rlwimi	r11, r10, 0, 27, 27
+	/* Insert the WriteThru flag into the TWC from the Linux PTE.
+	 * It is bit 25 in the Linux PTE and bit 30 in the TWC
+	 */
+	rlwimi	r11, r10, 32-5, 30, 30
 	DO_8xx_CPU6(0x3b80, r3)
 	mtspr	SPRN_MD_TWC, r11
 
@@ -559,6 +563,10 @@ DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR
 	 * It is bit 27 of both the Linux PTE and the TWC
 	 */
 	rlwimi	r11, r10, 0, 27, 27
+	/* Insert the WriteThru flag into the TWC from the Linux PTE.
+	 * It is bit 25 in the Linux PTE and bit 30 in the TWC
+	 */
+	rlwimi	r11, r10, 32-5, 30, 30
 	DO_8xx_CPU6(0x3b80, r3)
 	mtspr	SPRN_MD_TWC, r11
 	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
-- 
1.6.4.4

[PATCH 7/8] 8xx: start using dcbX instructions in various copy routines

[Joakim Tjernlund]

Now that 8xx can fixup dcbX instructions, start using them
where possible like every other PowerPc arch do.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/kernel/misc_32.S |   18 ------------------
 arch/powerpc/lib/copy_32.S    |   24 ------------------------
 2 files changed, 0 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index da9c0c4..8649f53 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -502,15 +502,7 @@ _GLOBAL(clear_pages)
 	li	r0,PAGE_SIZE/L1_CACHE_BYTES
 	slw	r0,r0,r4
 	mtctr	r0
-#ifdef CONFIG_8xx
-	li	r4, 0
-1:	stw	r4, 0(r3)
-	stw	r4, 4(r3)
-	stw	r4, 8(r3)
-	stw	r4, 12(r3)
-#else
 1:	dcbz	0,r3
-#endif
 	addi	r3,r3,L1_CACHE_BYTES
 	bdnz	1b
 	blr
@@ -535,15 +527,6 @@ _GLOBAL(copy_page)
 	addi	r3,r3,-4
 	addi	r4,r4,-4
 
-#ifdef CONFIG_8xx
-	/* don't use prefetch on 8xx */
-    	li	r0,4096/L1_CACHE_BYTES
-	mtctr	r0
-1:	COPY_16_BYTES
-	bdnz	1b
-	blr
-
-#else	/* not 8xx, we can prefetch */
 	li	r5,4
 
 #if MAX_COPY_PREFETCH > 1
@@ -584,7 +567,6 @@ _GLOBAL(copy_page)
 	li	r0,MAX_COPY_PREFETCH
 	li	r11,4
 	b	2b
-#endif	/* CONFIG_8xx */
 
 /*
  * void atomic_clear_mask(atomic_t mask, atomic_t *addr)
diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
index c657de5..74a7f41 100644
--- a/arch/powerpc/lib/copy_32.S
+++ b/arch/powerpc/lib/copy_32.S
@@ -98,20 +98,7 @@ _GLOBAL(cacheable_memzero)
 	bdnz	4b
 3:	mtctr	r9
 	li	r7,4
-#if !defined(CONFIG_8xx)
 10:	dcbz	r7,r6
-#else
-10:	stw	r4, 4(r6)
-	stw	r4, 8(r6)
-	stw	r4, 12(r6)
-	stw	r4, 16(r6)
-#if CACHE_LINE_SIZE >= 32
-	stw	r4, 20(r6)
-	stw	r4, 24(r6)
-	stw	r4, 28(r6)
-	stw	r4, 32(r6)
-#endif /* CACHE_LINE_SIZE */
-#endif
 	addi	r6,r6,CACHELINE_BYTES
 	bdnz	10b
 	clrlwi	r5,r8,32-LG_CACHELINE_BYTES
@@ -200,9 +187,7 @@ _GLOBAL(cacheable_memcpy)
 	mtctr	r0
 	beq	63f
 53:
-#if !defined(CONFIG_8xx)
 	dcbz	r11,r6
-#endif
 	COPY_16_BYTES
 #if L1_CACHE_BYTES >= 32
 	COPY_16_BYTES
@@ -356,14 +341,6 @@ _GLOBAL(__copy_tofrom_user)
 	li	r11,4
 	beq	63f
 
-#ifdef CONFIG_8xx
-	/* Don't use prefetch on 8xx */
-	mtctr	r0
-	li	r0,0
-53:	COPY_16_BYTES_WITHEX(0)
-	bdnz	53b
-
-#else /* not CONFIG_8xx */
 	/* Here we decide how far ahead to prefetch the source */
 	li	r3,4
 	cmpwi	r0,1
@@ -416,7 +393,6 @@ _GLOBAL(__copy_tofrom_user)
 	li	r3,4
 	li	r7,0
 	bne	114b
-#endif /* CONFIG_8xx */
 
 63:	srwi.	r0,r5,2
 	mtctr	r0
-- 
1.6.4.4

[PATCH 8/8] 8xx: Remove DIRTY pte handling in DTLB Error.

[Joakim Tjernlund]

There is no need to do set the DIRTY bit directly in DTLB Error.
Trap to do_page_fault() and let the generic MM code do the work.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/kernel/head_8xx.S |   96 ----------------------------------------
 1 files changed, 0 insertions(+), 96 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 65e0526..d6d70f2 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -500,102 +500,6 @@ DataTLBError:
 	cmpwi	cr0, r10, 0x00f0
 	beq-	FixupDAR	/* must be a buggy dcbX, icbi insn. */
 DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR */
-	mfspr	r11, SPRN_DSISR
-	/* As the DAR fixup may clear store we may have all 3 states zero.
-	 * Make sure only 0x0200(store) falls down into DIRTY handling
-	 */
-	andis.	r11, r11, 0x4a00	/* !translation, protection or store */
-	srwi	r11, r11, 16
-	cmpwi	cr0, r11, 0x0200	/* just store ? */
-	bne	2f
-	/* Only Change bit left now, do it here as it is faster
-	 * than trapping to the C fault handler.
-	*/
-
-	/* The EA of a data TLB miss is automatically stored in the MD_EPN
-	 * register.  The EA of a data TLB error is automatically stored in
-	 * the DAR, but not the MD_EPN register.  We must copy the 20 most
-	 * significant bits of the EA from the DAR to MD_EPN before we
-	 * start walking the page tables.  We also need to copy the CASID
-	 * value from the M_CASID register.
-	 * Addendum:  The EA of a data TLB error is _supposed_ to be stored
-	 * in DAR, but it seems that this doesn't happen in some cases, such
-	 * as when the error is due to a dcbi instruction to a page with a
-	 * TLB that doesn't have the changed bit set.  In such cases, there
-	 * does not appear to be any way  to recover the EA of the error
-	 * since it is neither in DAR nor MD_EPN.  As a workaround, the
-	 * _PAGE_HWWRITE bit is set for all kernel data pages when the PTEs
-	 * are initialized in mapin_ram().  This will avoid the problem,
-	 * assuming we only use the dcbi instruction on kernel addresses.
-	 */
-
-	/* DAR is in r10 already */
-	rlwinm	r11, r10, 0, 0, 19
-	ori	r11, r11, MD_EVALID
-	mfspr	r10, SPRN_M_CASID
-	rlwimi	r11, r10, 0, 28, 31
-	DO_8xx_CPU6(0x3780, r3)
-	mtspr	SPRN_MD_EPN, r11
-
-	mfspr	r10, SPRN_M_TWB	/* Get level 1 table entry address */
-
-	/* If we are faulting a kernel address, we have to use the
-	 * kernel page tables.
-	 */
-	andi.	r11, r10, 0x0800
-	beq	3f
-	lis	r11, swapper_pg_dir@h
-	ori	r11, r11, swapper_pg_dir@l
-	rlwimi	r10, r11, 0, 2, 19
-3:
-	lwz	r11, 0(r10)	/* Get the level 1 entry */
-	rlwinm.	r10, r11,0,0,19	/* Extract page descriptor page address */
-	beq	2f		/* If zero, bail */
-
-	/* We have a pte table, so fetch the pte from the table.
-	 */
-	ori	r11, r11, 1		/* Set valid bit in physical L2 page */
-	DO_8xx_CPU6(0x3b80, r3)
-	mtspr	SPRN_MD_TWC, r11	/* Load pte table base address */
-	mfspr	r10, SPRN_MD_TWC	/* ....and get the pte address */
-	lwz	r10, 0(r10)		/* Get the pte */
-	/* Insert the Guarded flag into the TWC from the Linux PTE.
-	 * It is bit 27 of both the Linux PTE and the TWC
-	 */
-	rlwimi	r11, r10, 0, 27, 27
-	/* Insert the WriteThru flag into the TWC from the Linux PTE.
-	 * It is bit 25 in the Linux PTE and bit 30 in the TWC
-	 */
-	rlwimi	r11, r10, 32-5, 30, 30
-	DO_8xx_CPU6(0x3b80, r3)
-	mtspr	SPRN_MD_TWC, r11
-	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
-
-	ori	r10, r10, _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_HWWRITE
-	stw	r10, 0(r11)		/* and update pte in table */
-	xori	r10, r10, _PAGE_RW	/* RW bit is inverted */
-
-	/* The Linux PTE won't go exactly into the MMU TLB.
-	 * Software indicator bits 22 and 28 must be clear.
-	 * Software indicator bits 24, 25, 26, and 27 must be
-	 * set.  All other Linux PTE bits control the behavior
-	 * of the MMU.
-	 */
-	li	r11, 0x00f0
-	mtspr	SPRN_DAR,r11	/* Tag DAR */
-	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
-	DO_8xx_CPU6(0x3d80, r3)
-	mtspr	SPRN_MD_RPN, r10	/* Update TLB entry */
-
-	mfspr	r10, SPRN_M_TW	/* Restore registers */
-	lwz	r11, 0(r0)
-	mtcr	r11
-	lwz	r11, 4(r0)
-#ifdef CONFIG_8xx_CPU6
-	lwz	r3, 8(r0)
-#endif
-	rfi
-2:
 	mfspr	r10, SPRN_M_TW	/* Restore registers */
 	lwz	r11, 0(r0)
 	mtcr	r11
-- 
1.6.4.4

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

On Wed, Nov 04, 2009 at 02:38:32PM +0100, Joakim Tjernlund wrote:
> Here is the latest(last?) round of this series. I
> hope I got everything right now.
> Scott and Rex, please test and send ACK/NACK.
> 
>   Jocke
> 
> Joakim Tjernlund (8):
>   8xx: invalidate non present TLBs

This works, and is an important fix -- it should be applied even if the rest
of the patchset isn't ready.

>   8xx: Update TLB asm so it behaves as linux mm expects.
>   8xx: Tag DAR with 0x00f0 to catch buggy instructions.

Up through this point works.

>   8xx: Fixup DAR from buggy dcbX instructions.

With this, the kernel hangs after "Mount-cache hash table entries: 512".

>   8xx: Add missing Guarded setting in DTLB Error.
>   8xx: Restore _PAGE_WRITETHRU
>   8xx: start using dcbX instructions in various copy routines

Once I get up to this patch, it no longer hangs, but I get some user
processes consistently failing with EFAULT.

>   8xx: Remove DIRTY pte handling in DTLB Error.

No change after this patch.

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 06/11/2009 01:33:05:
>
> On Wed, Nov 04, 2009 at 02:38:32PM +0100, Joakim Tjernlund wrote:
> > Here is the latest(last?) round of this series. I
> > hope I got everything right now.
> > Scott and Rex, please test and send ACK/NACK.
> >
> >   Jocke
> >
> > Joakim Tjernlund (8):
> >   8xx: invalidate non present TLBs
>
> This works, and is an important fix -- it should be applied even if the rest
> of the patchset isn't ready.

True.

>
> >   8xx: Update TLB asm so it behaves as linux mm expects.

I think this is ready too.

> >   8xx: Tag DAR with 0x00f0 to catch buggy instructions.
>
> Up through this point works.

hmm, here tagging of DAR is in place, do you ever hit the
page fault handler with address == 0x00f0? If you do,
the kernel somehow manges to fix it instead of erroring out.

I do notice one thing, I forgot to add the CPU6 errata to
the DAR tagging. Are you using the CPU6 errata?

>
> >   8xx: Fixup DAR from buggy dcbX instructions.
>
> With this, the kernel hangs after "Mount-cache hash table entries: 512".

Somewhat surprising result. I didn't expect you would even hit this
condition now as we haven't enabled the use of dcbX insn yet.
The only thing I can think of is the you hit the 0x00f0 due to other
dcbX insn use and the kernel managed to fixup this in the page fault handler
by pure luck before.

The only thing I can thing of being wrong here is your suggested fix:
+       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
+       ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
+       rlwimi  r11, r10, 22, 0xffc

What if you change that back to what worked for you before:
 lis     r11, swapper_pg_dir@h
 ori     r11, r11, swapper_pg_dir@l
 rlwinm  r11, r11, 0, 0x3ffff000
 rlwimi  r11, r10, 22, 0xffc
or possibly
 lis		 r11, swapper_pg_dir@h
 ori		 r11, r11, swapper_pg_dir@l
 subis	 r11, r11, PAGE_OFFSET@h
 rlwimi	 r11, r10, 22, 0xffc

hmm, some missed CPU6 errata calls in DARFixup too.
>
> >   8xx: Add missing Guarded setting in DTLB Error.
> >   8xx: Restore _PAGE_WRITETHRU
> >   8xx: start using dcbX instructions in various copy routines
>
> Once I get up to this patch, it no longer hangs, but I get some user
> processes consistently failing with EFAULT.

No surprise as the it seems like the DAR decoding is broken.

>
> >   8xx: Remove DIRTY pte handling in DTLB Error.
>
> No change after this patch.
>
> -Scott
>
>

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

>
> Scott Wood <scottwood@freescale.com> wrote on 06/11/2009 01:33:05:
> >
> > On Wed, Nov 04, 2009 at 02:38:32PM +0100, Joakim Tjernlund wrote:
> > > Here is the latest(last?) round of this series. I
> > > hope I got everything right now.
> > > Scott and Rex, please test and send ACK/NACK.
> > >
> > >   Jocke
> > >
> > > Joakim Tjernlund (8):
> > >   8xx: invalidate non present TLBs
> >
> > This works, and is an important fix -- it should be applied even if the rest
> > of the patchset isn't ready.
>
> True.
>
> >
> > >   8xx: Update TLB asm so it behaves as linux mm expects.
>
> I think this is ready too.
>
> > >   8xx: Tag DAR with 0x00f0 to catch buggy instructions.
> >
> > Up through this point works.
>
> hmm, here tagging of DAR is in place, do you ever hit the
> page fault handler with address == 0x00f0? If you do,
> the kernel somehow manges to fix it instead of erroring out.
>
> I do notice one thing, I forgot to add the CPU6 errata to
> the DAR tagging. Are you using the CPU6 errata?

DAR isn't affected by CPU6 so this should not be a problem.

>
> >
> > >   8xx: Fixup DAR from buggy dcbX instructions.
> >
> > With this, the kernel hangs after "Mount-cache hash table entries: 512".
>
> Somewhat surprising result. I didn't expect you would even hit this
> condition now as we haven't enabled the use of dcbX insn yet.
> The only thing I can think of is the you hit the 0x00f0 due to other
> dcbX insn use and the kernel managed to fixup this in the page fault handler
> by pure luck before.
>
> The only thing I can thing of being wrong here is your suggested fix:
> +       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
> +       ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
> +       rlwimi  r11, r10, 22, 0xffc
>
> What if you change that back to what worked for you before:
>  lis     r11, swapper_pg_dir@h
>  ori     r11, r11, swapper_pg_dir@l
>  rlwinm  r11, r11, 0, 0x3ffff000
>  rlwimi  r11, r10, 22, 0xffc
> or possibly
>  lis       r11, swapper_pg_dir@h
>  ori       r11, r11, swapper_pg_dir@l
>  subis    r11, r11, PAGE_OFFSET@h
>  rlwimi    r11, r10, 22, 0xffc
>
> hmm, some missed CPU6 errata calls in DARFixup too.

Same here, not a problem

I did notice a bug that has been there a long time so
I don't think it is the problem:
+       add     r10, r10, r25   ;b      151f
+       add     r10, r10, r25   ;b      151f
should be r26 instead:
+       add     r10, r10, r25   ;b      151f
+       add     r10, r10, r26   ;b      151f

 Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

On Mon, Nov 09, 2009 at 03:53:21PM -0600, Scott Wood wrote:
> On Fri, Nov 06, 2009 at 10:29:44AM +0100, Joakim Tjernlund wrote:
> > > > With this, the kernel hangs after "Mount-cache hash table entries: 512".
> > >
> > > Somewhat surprising result. I didn't expect you would even hit this
> > > condition now as we haven't enabled the use of dcbX insn yet.
> > > The only thing I can think of is the you hit the 0x00f0 due to other
> > > dcbX insn use and the kernel managed to fixup this in the page fault handler
> > > by pure luck before.
> 
> It's bizarre -- it still happens even if I revert the branch to FixupDAR. 
> However, if I comment out the final "b 151b", it stops happening.  It also
> stops happening sometimes depending on where I stick printk()s to debug
> this.
> 
> So it may be an unrelated issue that just got perturbed somehow.

OK, figured it out.  The fixup code pushed things around so that in
syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page
boundary, with the rfi after the page boundary.  On crossing the boundary,
we take an ITLB miss (which goes from possibility to certainty with the
CPU15 workaround), and SRR0/SRR1 get clobbered.

I suppose we'll need to find all places where we do rfi with the MMU
enabled, and ensure acceptable alignment. :-(

Either that, or require that the kernel text be pinned, though that does not
interact well with CPU15.

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 00:00:04:
>
> On Mon, Nov 09, 2009 at 03:53:21PM -0600, Scott Wood wrote:
> > On Fri, Nov 06, 2009 at 10:29:44AM +0100, Joakim Tjernlund wrote:
> > > > > With this, the kernel hangs after "Mount-cache hash table entries: 512".
> > > >
> > > > Somewhat surprising result. I didn't expect you would even hit this
> > > > condition now as we haven't enabled the use of dcbX insn yet.
> > > > The only thing I can think of is the you hit the 0x00f0 due to other
> > > > dcbX insn use and the kernel managed to fixup this in the page fault handler
> > > > by pure luck before.
> >
> > It's bizarre -- it still happens even if I revert the branch to FixupDAR.
> > However, if I comment out the final "b 151b", it stops happening.  It also
> > stops happening sometimes depending on where I stick printk()s to debug
> > this.
> >
> > So it may be an unrelated issue that just got perturbed somehow.
>
> OK, figured it out.  The fixup code pushed things around so that in

Cool!

> syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page
> boundary, with the rfi after the page boundary.  On crossing the boundary,
> we take an ITLB miss (which goes from possibility to certainty with the
> CPU15 workaround), and SRR0/SRR1 get clobbered.

I am not familiar with CPU15 since we never had that problem.
The patch series is OK then, but one need to add some CPU15 love?

>
> I suppose we'll need to find all places where we do rfi with the MMU
> enabled, and ensure acceptable alignment. :-(

Ouch, but it can't be that many places though.

>
> Either that, or require that the kernel text be pinned, though that does not
> interact well with CPU15.

Why does not pinning interact well with CPU15? If pinned, you never get
a TLB miss for kernel text so that should mitigate the CPU15 problem.

      Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 00:00:04:
>> syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page
>> boundary, with the rfi after the page boundary.  On crossing the boundary,
>> we take an ITLB miss (which goes from possibility to certainty with the
>> CPU15 workaround), and SRR0/SRR1 get clobbered.
> 
> I am not familiar with CPU15 since we never had that problem.
> The patch series is OK then, but one need to add some CPU15 love?

It's not CPU15 itself that is causing the problem, but rather the 
workaround for CPU15 takes something that has a possibility of happening 
and makes it certain to happen.

>> Either that, or require that the kernel text be pinned, though that does not
>> interact well with CPU15.
> 
> Why does not pinning interact well with CPU15? If pinned, you never get
> a TLB miss for kernel text so that should mitigate the CPU15 problem.

The nature of the workaround for CPU15 is that we can't keep it pinned 
-- we have to take an ITLB miss on every page boundary crossing.  If you 
try to pin, it'll just be invalidated by the workaround.

There is an alternate workaround, but it requires compiler changes.

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Scott Wood wrote:
> Joakim Tjernlund wrote:
>> Why does not pinning interact well with CPU15? If pinned, you never get
>> a TLB miss for kernel text so that should mitigate the CPU15 problem.
> 
> The nature of the workaround for CPU15 is that we can't keep it pinned 
> -- we have to take an ITLB miss on every page boundary crossing.  If you 
> try to pin, it'll just be invalidated by the workaround.

Except that the invalidation only happens when you take an ITLB miss on 
an adjacent page, which means we'd likely never get CPU15 protection for 
kernel code if pinning is enabled. :-(

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 17:55:28:
>
> Scott Wood wrote:
> > Joakim Tjernlund wrote:
> >> Why does not pinning interact well with CPU15? If pinned, you never get
> >> a TLB miss for kernel text so that should mitigate the CPU15 problem.
> >
> > The nature of the workaround for CPU15 is that we can't keep it pinned
> > -- we have to take an ITLB miss on every page boundary crossing.  If you
> > try to pin, it'll just be invalidated by the workaround.
>
> Except that the invalidation only happens when you take an ITLB miss on
> an adjacent page, which means we'd likely never get CPU15 protection for
> kernel code if pinning is enabled. :-(

So tlbie invalidates pinned TLBs too?
It is likely that you won't get ITLB Misses for normal kernel
space but for modules you will. Also, since the pinned D&I TLBs overlap,
how do you make sure that invalidating a kernel DTLB won't
spill over to the pinned ITLB?

Does pinned TLBs work for you?
Rex, how does it look in your end?

 Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 00:00:04:
>
> On Mon, Nov 09, 2009 at 03:53:21PM -0600, Scott Wood wrote:
> > On Fri, Nov 06, 2009 at 10:29:44AM +0100, Joakim Tjernlund wrote:
> > > > > With this, the kernel hangs after "Mount-cache hash table entries: 512".
> > > >
> > > > Somewhat surprising result. I didn't expect you would even hit this
> > > > condition now as we haven't enabled the use of dcbX insn yet.
> > > > The only thing I can think of is the you hit the 0x00f0 due to other
> > > > dcbX insn use and the kernel managed to fixup this in the page fault handler
> > > > by pure luck before.
> >
> > It's bizarre -- it still happens even if I revert the branch to FixupDAR.
> > However, if I comment out the final "b 151b", it stops happening.  It also
> > stops happening sometimes depending on where I stick printk()s to debug
> > this.
> >
> > So it may be an unrelated issue that just got perturbed somehow.
>
> OK, figured it out.  The fixup code pushed things around so that in
> syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page
> boundary, with the rfi after the page boundary.  On crossing the boundary,
> we take an ITLB miss (which goes from possibility to certainty with the
> CPU15 workaround), and SRR0/SRR1 get clobbered.

I think I have misunderstood, its is not CPU15 or 8xx problem per se, it
is a general problem that could happen to any ppc CPU, right?
8xx just happen to be the first CPU that hits this case due to my
DAR fixing

>
> I suppose we'll need to find all places where we do rfi with the MMU
> enabled, and ensure acceptable alignment. :-(

That seems the right fix, the asm in entry_32.S needs have some alignment
here and there to make sure you don't cross a page boundary at the wrong time.

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 21:00:07:
>
> Joakim Tjernlund wrote:
> > I think I have misunderstood, its is not CPU15 or 8xx problem per se, it
> > is a general problem that could happen to any ppc CPU, right?
> > 8xx just happen to be the first CPU that hits this case due to my
> > DAR fixing
>
> Is there any chip other than 8xx where the kernel text is not always pinned?

No idea, something for the list to comment on.
But if so 8xx should do the same(at least for ITLB), there is
no other way to guarantee this problem won't happen I think.

 Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 21:27:05:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 17:55:28:
> >> Except that the invalidation only happens when you take an ITLB miss on
> >> an adjacent page, which means we'd likely never get CPU15 protection for
> >> kernel code if pinning is enabled. :-(
> >
> > So tlbie invalidates pinned TLBs too?
>
> Yes.

OK, and this is in no way unique for 8xx?

>
> > It is likely that you won't get ITLB Misses for normal kernel
> > space but for modules you will. Also, since the pinned D&I TLBs overlap,
> > how do you make sure that invalidating a kernel DTLB won't
> > spill over to the pinned ITLB?
>
> I don't see when you'd ever invalidate the pinned entry, whether for
> instruction or data purposes, unless you take an ITLB miss for the page
> immediately following the pinned mapping.

tlbie is used by the kernel in other places too, so I assumed it could
be on kernel space too. However, it was just a guess, and by the looks of things,
a bad one.

>
> > Does pinned TLBs work for you?
>
> Yes -- I turned on CONFIG_PIN_TLB, padded things so that the rfi is
> still on the beginning of a new page, and it boots fine.  If I keep
> everything the same but replace MD_RSV4I with zero, it fails again.

Ah, good.

>
> But who knows when CPU15 will strike...
yes, maybe there is a way around that. Perhaps by using one of the
pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?

In any case one should consider fixing
entry_32.S and friends to be properly aligned. Then we are no worse off
than when we started, right?

     Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 22:36:32:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 21:27:05:
> >> Joakim Tjernlund wrote:
> >>> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 17:55:28:
> >>>> Except that the invalidation only happens when you take an ITLB miss on
> >>>> an adjacent page, which means we'd likely never get CPU15 protection for
> >>>> kernel code if pinning is enabled. :-(
> >>> So tlbie invalidates pinned TLBs too?
> >> Yes.
> >
> > OK, and this is in no way unique for 8xx?
>
> Not sure about others, but 8xx manual explicitly says that it
> invalidates reserved entries.
>
> >> But who knows when CPU15 will strike...
> > yes, maybe there is a way around that. Perhaps by using one of the
> > pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
>
> Not sure what you mean...  loaded modules won't be pinned, and since
> they shouldn't contain rfi, don't need to be.

But CPU15 may invalidate a pinned TLB if you take a TLB Miss?
If not there should not be a problem, because the rest
of the kernel will never take a ITLB Miss.

>
> I don't see how to pin any part of the kernel without introducing some
> possibility of CPU15, unless we go scanning the last word of every
> instruction page, creating trampolines, and hoping there's no data
> embedded in the text segment. :-P

Yes, it is not going to be easy.
So aligning the srr0/srr1/rfi properly is needed.

BTW, you could probably cram the DARFix into the DTLBerror with some luck.
Especially if you allow it to spill over to the next trap. Then create a
branch insn at 0x1500 to 0x1600. Would that make everything aligned again?

  Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 22:36:32:
>> Joakim Tjernlund wrote:
>>> yes, maybe there is a way around that. Perhaps by using one of the
>>> pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
>> Not sure what you mean...  loaded modules won't be pinned, and since
>> they shouldn't contain rfi, don't need to be.
> 
> But CPU15 may invalidate a pinned TLB if you take a TLB Miss?
> If not there should not be a problem, because the rest
> of the kernel will never take a ITLB Miss.

It wasn't the CPU15 workaround that I was worried about taking down the 
pinning -- but rather the CPU15 bug itself causing bad code to be 
executed inside the pinned kernel mapping.

However, the erratum says "MMU page", not "4K region", so I suppose if 
we have a pinned 8M page the problem could only occur at the end of the 
8M (by which point the text segment should have ended).

Unless we have any evidence that this is not what the erratum means, I'd 
say make pinning mandatory, and avoid placing modules immediately after 
a pinned entry.

> BTW, you could probably cram the DARFix into the DTLBerror with some luck.
> Especially if you allow it to spill over to the next trap. Then create a
> branch insn at 0x1500 to 0x1600. Would that make everything aligned again?

Yes, until some other code change breaks it again.

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 23:02:10:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 22:36:32:
> >> Joakim Tjernlund wrote:
> >>> yes, maybe there is a way around that. Perhaps by using one of the
> >>> pinned entries for loaded modules, i.e avoid ITLB misses for kernel space?
> >> Not sure what you mean...  loaded modules won't be pinned, and since
> >> they shouldn't contain rfi, don't need to be.
> >
> > But CPU15 may invalidate a pinned TLB if you take a TLB Miss?
> > If not there should not be a problem, because the rest
> > of the kernel will never take a ITLB Miss.
>
> It wasn't the CPU15 workaround that I was worried about taking down the
> pinning -- but rather the CPU15 bug itself causing bad code to be
> executed inside the pinned kernel mapping.

Oh, but then one is screwed anyway.

>
> However, the erratum says "MMU page", not "4K region", so I suppose if
> we have a pinned 8M page the problem could only occur at the end of the
> 8M (by which point the text segment should have ended).

The wording makes me wonder why not a dcbi would fix the problem too.
Invalidate the problem cache lines and let the branch resolve.

>
> Unless we have any evidence that this is not what the erratum means, I'd
> say make pinning mandatory, and avoid placing modules immediately after
> a pinned entry.

It is worth a try.

>
> > BTW, you could probably cram the DARFix into the DTLBerror with some luck.
> > Especially if you allow it to spill over to the next trap. Then create a
> > branch insn at 0x1500 to 0x1600. Would that make everything aligned again?
>
> Yes, until some other code change breaks it again.
>
> -Scott
>
>

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 23:02:10:
>> Joakim Tjernlund wrote:
>> It wasn't the CPU15 workaround that I was worried about taking down the
>> pinning -- but rather the CPU15 bug itself causing bad code to be
>> executed inside the pinned kernel mapping.
> 
> Oh, but then one is screwed anyway.

Not if there's a workaround...

>> However, the erratum says "MMU page", not "4K region", so I suppose if
>> we have a pinned 8M page the problem could only occur at the end of the
>> 8M (by which point the text segment should have ended).
> 
> The wording makes me wonder why not a dcbi would fix the problem too.
> Invalidate the problem cache lines and let the branch resolve.

Where would you put the dcbi?  How do you regain control after that 
cache line has been refilled, but before code flows back to the bad branch?

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 10/11/2009 23:02:10:
> >> Joakim Tjernlund wrote:
> >> It wasn't the CPU15 workaround that I was worried about taking down the
> >> pinning -- but rather the CPU15 bug itself causing bad code to be
> >> executed inside the pinned kernel mapping.
> >
> > Oh, but then one is screwed anyway.
>
> Not if there's a workaround...
>
> >> However, the erratum says "MMU page", not "4K region", so I suppose if
> >> we have a pinned 8M page the problem could only occur at the end of the
> >> 8M (by which point the text segment should have ended).
> >
> > The wording makes me wonder why not a dcbi would fix the problem too.
> > Invalidate the problem cache lines and let the branch resolve.
>
> Where would you put the dcbi?  How do you regain control after that
> cache line has been refilled, but before code flows back to the bad branch?

The dcbi would replace the current CPU15 tlbie.

I would try mitigating bullet 3 (unresolved long enough ...) by
invalidating n+1 cache line. Hopefully that will abort the prefetch.

If that doesn't work, invalidate n+2(bullet 4) as it says it must be in cache for the problem
to manifest.

But I see now that I probably misread the errata so the above wont work.

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

On Wed, Nov 11, 2009 at 01:06:10AM +0100, Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
> > Where would you put the dcbi?  How do you regain control after that
> > cache line has been refilled, but before code flows back to the bad branch?
> 
> The dcbi would replace the current CPU15 tlbie.

But that only works if you take an ITLB miss at the right time.

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 16:26:53:
>
> On Wed, Nov 11, 2009 at 01:06:10AM +0100, Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
> > > Where would you put the dcbi?  How do you regain control after that
> > > cache line has been refilled, but before code flows back to the bad branch?
> >
> > The dcbi would replace the current CPU15 tlbie.
>
> But that only works if you take an ITLB miss at the right time.

Yeah, I misread the CPU15 errata so my ideas will not work.

Anyhow, will you send a patch that make TLB pinning mandatory?
After that my series can go in.

        Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 16:26:53:
>> On Wed, Nov 11, 2009 at 01:06:10AM +0100, Joakim Tjernlund wrote:
>>> Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
>>>> Where would you put the dcbi?  How do you regain control after that
>>>> cache line has been refilled, but before code flows back to the bad branch?
>>> The dcbi would replace the current CPU15 tlbie.
>> But that only works if you take an ITLB miss at the right time.
> 
> Yeah, I misread the CPU15 errata so my ideas will not work.
> 
> Anyhow, will you send a patch that make TLB pinning mandatory?
> After that my series can go in.

One other concern with pinning on 8xx -- could it cause problems with 
uncached DMA mappings?  What happens if a speculative load pulls in a 
cache line in an area that's supposed to be uncached?

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 12/11/2009 20:45:59:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 16:26:53:
> >> On Wed, Nov 11, 2009 at 01:06:10AM +0100, Joakim Tjernlund wrote:
> >>> Scott Wood <scottwood@freescale.com> wrote on 11/11/2009 00:21:18:
> >>>> Where would you put the dcbi?  How do you regain control after that
> >>>> cache line has been refilled, but before code flows back to the bad branch?
> >>> The dcbi would replace the current CPU15 tlbie.
> >> But that only works if you take an ITLB miss at the right time.
> >
> > Yeah, I misread the CPU15 errata so my ideas will not work.
> >
> > Anyhow, will you send a patch that make TLB pinning mandatory?
> > After that my series can go in.
>
> One other concern with pinning on 8xx -- could it cause problems with
> uncached DMA mappings?  What happens if a speculative load pulls in a
> cache line in an area that's supposed to be uncached?

hmm, why should this be a problem? Pinning has been around as a config option
for a long time so any problems should have surfaced by now.

Secondly, I was thinking that we could just make the ITLB pinning
mandatory and let the DTLB pinning be as is, configurable.

 Jocke

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 12/11/2009 20:45:59:
>> One other concern with pinning on 8xx -- could it cause problems with
>> uncached DMA mappings?  What happens if a speculative load pulls in a
>> cache line in an area that's supposed to be uncached?
> 
> hmm, why should this be a problem?

Because then you would be accessing potentially stale DMA data -- and 
more generally, the architecture prohibits such mixing.

> Pinning has been around as a config option
> for a long time so any problems should have surfaced by now.

It has existed as an option, which is going to get less test coverage 
than something that is always on.  Plus, it would not be a particularly 
common failure -- easy to blame one-off weirdness on something else.

> Secondly, I was thinking that we could just make the ITLB pinning
> mandatory and let the DTLB pinning be as is, configurable.

That could work.  We could also limit the pool of memory 
dma_alloc_coherent() uses to not overlap with anything pinned.

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 12/11/2009 22:57:59:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 12/11/2009 20:45:59:
> >> One other concern with pinning on 8xx -- could it cause problems with
> >> uncached DMA mappings?  What happens if a speculative load pulls in a
> >> cache line in an area that's supposed to be uncached?
> >
> > hmm, why should this be a problem?
>
> Because then you would be accessing potentially stale DMA data -- and
> more generally, the architecture prohibits such mixing.
>
> > Pinning has been around as a config option
> > for a long time so any problems should have surfaced by now.
>
> It has existed as an option, which is going to get less test coverage
> than something that is always on.  Plus, it would not be a particularly
> common failure -- easy to blame one-off weirdness on something else.
>
> > Secondly, I was thinking that we could just make the ITLB pinning
> > mandatory and let the DTLB pinning be as is, configurable.
>
> That could work.  We could also limit the pool of memory
> dma_alloc_coherent() uses to not overlap with anything pinned.

And try using the remaing 3 ITLBs which are unused when pinning to
cover the module space. Probably not going to be perfect.

Anyhow, lets start simple and just do the pinned ITLB so the
new TLB code can be applied. Can you confirm this works for you?

>From 0c30d6c1ee45341fcfc5643bd2ba876e3c9a416e Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Fri, 13 Nov 2009 00:26:59 +0100
Subject: [PATCH] 8xx: Always pin kernel instruction TLB

---
 arch/powerpc/kernel/head_8xx.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index adc5a32..c7a4e60 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -767,7 +767,7 @@ start_here:
  */
 initial_mmu:
 	tlbia			/* Invalidate all TLB entries */
-#ifdef CONFIG_PIN_TLB
+#ifdef 1 /* CONFIG_PIN_TLB */
 	lis	r8, MI_RSV4I@h
 	ori	r8, r8, 0x1c00
 #else
--
1.6.4.4

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Scott Wood]

Joakim Tjernlund wrote:
> Anyhow, lets start simple and just do the pinned ITLB so the
> new TLB code can be applied. Can you confirm this works for you?

It works (after changing #ifdef 1 to #if 1).

-Scott

Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 13/11/2009 20:25:48:
>
> Joakim Tjernlund wrote:
> > Anyhow, lets start simple and just do the pinned ITLB so the
> > new TLB code can be applied. Can you confirm this works for you?
>
> It works (after changing #ifdef 1 to #if 1).

OK, new series sent.

BTW, one can probably avoid the extra space by the work around if the first
part(up to and including the insn check) is included in the DTLB error
and the second half is put just before the . = 0x2000, you would have to
use the self modifying variant though.
Not something I am going to play with.

 Jocke