linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] 8xx MMU fixes
@ 2009-10-08 13:24 Joakim Tjernlund
  2009-10-08 13:24 ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Joakim Tjernlund
  2009-10-09  0:15 ` [PATCH 0/6] 8xx MMU fixes Rex Feany
  0 siblings, 2 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

So here we go again. This time I am
fairly confindent I got most things correct :)
Also manged to use even less instructions in the
TLB Miss handlers.

Scott and Rex, forget previous versions and
try this one out.

Once this works we can discuss further enchantments.

Joakim Tjernlund (6):
  8xx: DTLB Error must check for more errors.
  8xx: Update TLB asm so it behaves as linux mm expects.
  8xx: invalidate non present TLBs
  8xx: Tag DAR with 0x00f0 to catch buggy instructions.
  8xx: Fixup DAR from buggy dcbX instructions.
  8xx: start using dcbX instructions in various copy routines

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/6] 8xx: DTLB Error must check for more errors.
  2009-10-08 13:24 [PATCH 0/6] 8xx MMU fixes Joakim Tjernlund
@ 2009-10-08 13:24 ` Joakim Tjernlund
  2009-10-08 13:24   ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Joakim Tjernlund
  2009-10-08 20:48   ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Benjamin Herrenschmidt
  2009-10-09  0:15 ` [PATCH 0/6] 8xx MMU fixes Rex Feany
  1 sibling, 2 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

DataTLBError currently does:
 if ((err & 0x02000000) == 0)
    DSI();
This won't handle a store with no valid translation.
Change this to
 if ((err & 0x48000000) != 0)
    DSI();
that is, branch to DSI if either !permission or
!translation.
---
 arch/powerpc/kernel/head_8xx.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 52ff8c5..118bb05 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -472,8 +472,8 @@ DataTLBError:
 	/* First, make sure this was a store operation.
 	*/
 	mfspr	r10, SPRN_DSISR
-	andis.	r11, r10, 0x0200	/* If set, indicates store op */
-	beq	2f
+	andis.	r11, r10, 0x4800 /* no translation, no permission. */
+	bne	2f	/* branch if either is set */
 
 	/* 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
-- 
1.6.4.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
  2009-10-08 13:24 ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Joakim Tjernlund
@ 2009-10-08 13:24   ` Joakim Tjernlund
  2009-10-08 13:24     ` [PATCH 3/6] 8xx: invalidate non present TLBs Joakim Tjernlund
  2009-10-08 21:04     ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Benjamin Herrenschmidt
  2009-10-08 20:48   ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Benjamin Herrenschmidt
  1 sibling, 2 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

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 ?)
 - Less instructions in I/D TLB Miss.
 - Prepared for HWEXEC support.
 - Prepared kernel RO/user NA support.

Cons:
 - None ?
---
 arch/powerpc/include/asm/pte-8xx.h |   13 ++---
 arch/powerpc/kernel/head_8xx.S     |   93 ++++++++++++++++++------------------
 2 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
index 8c6e312..f23cd15 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -32,22 +32,21 @@
 #define _PAGE_FILE	0x0002	/* when !present: nonlinear file mapping */
 #define _PAGE_NO_CACHE	0x0002	/* I: cache inhibit */
 #define _PAGE_SHARED	0x0004	/* No ASID (context) compare */
+#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_EXEC	0x0008	/* software: i-cache coherency required */
 #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 118bb05..1639d16 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -333,26 +333,21 @@ 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
+	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
+	rlwimi.	r10, r10, 27, 31, 31
+	beq-	cr0, 2f /* Can be removed, costs a ITLB Err */
 
+#if 0 	/* Dont' bother with PP lsb, bit 21 for now */
+	/* r10 = (r10 & ~0x0400) | ((r10 & _PAGE_EXEC) << 7) */
+	rlwimi	r10, r10, 7, 21, 21 /* Set _PAGE_EXEC << 7 */
+#endif
 	/* 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 +360,22 @@ InstructionTLBMiss:
 	lwz	r3, 8(r0)
 #endif
 	rfi
+2:
+	mfspr	r11, SRR1
+	/* clear all error bits as TLB Miss
+	 * sets a few unconditionally
+	*/
+	rlwinm	r11, r11, 0, 0xffff
+	mtspr	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 +420,22 @@ 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
+	/* Need to know if load/store -> force a TLB Error
+	 * by copying ACCESSED to PRESENT.
+	*/
+	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
+	rlwimi	r10, r10, 27, 31, 31
+
+#if 0 /* Not yet */
+	/* Honour kernel RO, User NA */
+	andi.	r11, r10, _PAGE_USER | _PAGE_RW
+	bne-	cr0, 5f
+	ori	r10,r10, 0x200 /* Extended encoding, bit 22 */
 #endif
-	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
-	stw	r10, 0(r11)
+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 +481,12 @@ DataTLBError:
 	stw	r10, 0(r0)
 	stw	r11, 4(r0)
 
-	/* First, make sure this was a store operation.
-	*/
-	mfspr	r10, SPRN_DSISR
-	andis.	r11, r10, 0x4800 /* no translation, no permission. */
+	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.
+	*/
 
 	/* 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 +535,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/6] 8xx: invalidate non present TLBs
  2009-10-08 13:24   ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Joakim Tjernlund
@ 2009-10-08 13:24     ` Joakim Tjernlund
  2009-10-08 13:24       ` [PATCH 4/6] 8xx: Tag DAR with 0x00f0 to catch buggy instructions Joakim Tjernlund
  2009-10-08 21:04     ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Benjamin Herrenschmidt
  1 sibling, 1 reply; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

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.
---
 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 7699394..72941c7 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -39,7 +39,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)
@@ -243,6 +243,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);
+
         /* 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/6] 8xx: Tag DAR with 0x00f0 to catch buggy instructions.
  2009-10-08 13:24     ` [PATCH 3/6] 8xx: invalidate non present TLBs Joakim Tjernlund
@ 2009-10-08 13:24       ` Joakim Tjernlund
  2009-10-08 13:24         ` [PATCH 5/6] 8xx: Fixup DAR from buggy dcbX instructions Joakim Tjernlund
  0 siblings, 1 reply; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

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().
---
 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 1639d16..9707dc4 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
@@ -441,6 +447,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 */
@@ -481,6 +488,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 */
@@ -504,7 +515,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
@@ -546,6 +558,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/6] 8xx: Fixup DAR from buggy dcbX instructions.
  2009-10-08 13:24       ` [PATCH 4/6] 8xx: Tag DAR with 0x00f0 to catch buggy instructions Joakim Tjernlund
@ 2009-10-08 13:24         ` Joakim Tjernlund
  2009-10-08 13:24           ` [PATCH 6/6] 8xx: start using dcbX instructions in various copy routines Joakim Tjernlund
  0 siblings, 1 reply; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

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).
---
 arch/powerpc/kernel/head_8xx.S |  146 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 145 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 9707dc4..6541855 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -490,7 +490,8 @@ DataTLBError:
 
 	mfspr	r10, SPRN_DAR
 	cmpwi	cr0, r10, 0x00f0
-	beq-	2f	/* must be a buggy dcbX, icbi insn. */
+	beq-	FixDAR	/* must be a buggy dcbX, icbi insn. */
+DARFix:	/* Return from dcbx instruction bug workaround, r10 holds value of DAR */
 
 	mfspr	r11, SPRN_DSISR
 	andis.	r11, r11, 0x4800	/* !translation or protection */
@@ -600,6 +601,149 @@ 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 NO_SELF_MODIFYING_CODE /* define if you don't want to use self modifying code */
+	nop	/* A few nops to make the modified_instr: space below cache line aligned */
+	nop
+139:	/* fetch instruction from userspace memory */
+	DO_8xx_CPU6(0x3780, r3)
+	mtspr	SPRN_MD_EPN, r10
+	mfspr	r11, SPRN_M_TWB	/* Get level 1 table entry address */
+	lwz	r11, 0(r11)	/* Get the level 1 entry */
+	tophys  (r11, r11)
+	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
+	b	140f
+FixDAR:	/* Entry point for dcbx workaround. */
+	/* fetch instruction from memory. */
+	mfspr	r10, SPRN_SRR0
+	andis.	r11, r10, 0x8000
+	tophys  (r11, r10)
+	beq-	139b		/* Branch if user space address */
+140:	lwz	r11,0(r11)
+#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	DARFix		/* 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
+	add	r10, r10, r10
+	b	151f
+	add	r10, r10, r11
+	b	151f
+	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
+	mtctr	r11	/* r10 needs special handling */
+	b	154f
+	mtctr	r11	/* r11 needs special handling */
+	b	153f
+	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	DARFix			/* 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/6] 8xx: start using dcbX instructions in various copy routines
  2009-10-08 13:24         ` [PATCH 5/6] 8xx: Fixup DAR from buggy dcbX instructions Joakim Tjernlund
@ 2009-10-08 13:24           ` Joakim Tjernlund
  0 siblings, 0 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 13:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Rex Feany, Scott Wood

Now that 8xx can fixup dcbX instructions, start using them
where possible like every other PowerPc arch do.
---
 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 15f28e0..b92095e 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -495,15 +495,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
@@ -528,15 +520,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
@@ -577,7 +560,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] 8xx: DTLB Error must check for more errors.
  2009-10-08 13:24 ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Joakim Tjernlund
  2009-10-08 13:24   ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Joakim Tjernlund
@ 2009-10-08 20:48   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2009-10-08 20:48 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev, Rex Feany

On Thu, 2009-10-08 at 15:24 +0200, Joakim Tjernlund wrote:
> DataTLBError currently does:
>  if ((err & 0x02000000) == 0)
>     DSI();
> This won't handle a store with no valid translation.
> Change this to
>  if ((err & 0x48000000) != 0)
>     DSI();
> that is, branch to DSI if either !permission or
> !translation.
> ---

As I said earlier, I don't think this is necessary, just get rid of the
whole bunch of code in DataTLBError :-)

Ben.

>  arch/powerpc/kernel/head_8xx.S |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index 52ff8c5..118bb05 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -472,8 +472,8 @@ DataTLBError:
>  	/* First, make sure this was a store operation.
>  	*/
>  	mfspr	r10, SPRN_DSISR
> -	andis.	r11, r10, 0x0200	/* If set, indicates store op */
> -	beq	2f
> +	andis.	r11, r10, 0x4800 /* no translation, no permission. */
> +	bne	2f	/* branch if either is set */
>  
>  	/* 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
  2009-10-08 13:24   ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Joakim Tjernlund
  2009-10-08 13:24     ` [PATCH 3/6] 8xx: invalidate non present TLBs Joakim Tjernlund
@ 2009-10-08 21:04     ` Benjamin Herrenschmidt
  2009-10-08 22:44       ` Joakim Tjernlund
  1 sibling, 1 reply; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2009-10-08 21:04 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev, Rex Feany

On Thu, 2009-10-08 at 15:24 +0200, Joakim Tjernlund wrote:

> +#define _PAGE_RW	0x0400	/* lsb PP bits, inverted in HW */
> +#define _PAGE_USER	0x0800	/* msb PP bits */
>  
> +	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
> +	rlwimi.	r10, r10, 27, 31, 31
> +	beq-	cr0, 2f /* Can be removed, costs a ITLB Err */

Did you mean

+ 	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>4) */
+	rlwimi.	r10, r10, 28, 30, 31
+	beq-	cr0, 2f /* Can be removed, costs a ITLB Err */

Which would still be incorrect. You want -both- to be set,
and your code will move on if -any- is set. Might want to add
a ~ of the whole thing maybe and invert the condition ?

I find it easier to just do li rX, requested_bits, and then, andc.
rscratch, rX, rPTE

> +#if 0 	/* Dont' bother with PP lsb, bit 21 for now */
> +	/* r10 = (r10 & ~0x0400) | ((r10 & _PAGE_EXEC) << 7) */
> +	rlwimi	r10, r10, 7, 21, 21 /* Set _PAGE_EXEC << 7 */
> +#endif

I don't get that one. Don't bother with _PAGE_EXEC, there's no
control of execute permission that works on 8xx. It's #if 0 anyway.

You still need to massage the PP bits into place. I don't see that
happening.

As it is, your PTE contains for bit 20 and 21, which translates to:

   PTE:                 Translates to PP bits:
RW: 0   USER: 0          00  supervisor RW (ok)
RW: 0   USER: 1          01  supervisor RW user RO (WRONG)
RW: 1   USER: 0          10  supervisor RW user RW (WRONG)
RW: 1   USER: 1          11  supervisor RO user RO (WRONG)

I would suggest you do the stuff I suggested initially with RW and USER
being an "index" into a pre-cooked immediate value with all the encodings
which also gives you the extended encoding for supervisor RO for free.

> +	/* Need to know if load/store -> force a TLB Error
> +	 * by copying ACCESSED to PRESENT.
> +	*/
> +	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
> +	rlwimi	r10, r10, 27, 31, 31

That doesn't sound right, just like ITLB... you need to AND those two
bits in a way or another, or basically test that they are both set
(or neither is set)

> +#if 0 /* Not yet */
> +	/* Honour kernel RO, User NA */
> +	andi.	r11, r10, _PAGE_USER | _PAGE_RW
> +	bne-	cr0, 5f
> +	ori	r10,r10, 0x200 /* Extended encoding, bit 22 */
>  #endif
> -	mfspr	r11, SPRN_MD_TWC	/* get the pte address again */
> -	stw	r10, 0(r11)
> +5:	xori	r10, r10, _PAGE_RW  /* invert RW bit */

Well, you are missing that xori from the ITLB miss I think. Also, that
changes the table above to:

   PTE:                 Translates to PP bits: 
RW: 0   USER: 0          10  supervisor RW user RW (WRONG)
RW: 0   USER: 1          11  supervisor RO user RO (ok)
RW: 1   USER: 0          00  supervisor RW (ok)
RW: 1   USER: 1          01  supervisor RW user RO (WRONG)

So it's still not right :-)

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
  2009-10-08 21:04     ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Benjamin Herrenschmidt
@ 2009-10-08 22:44       ` Joakim Tjernlund
  2009-10-09  0:53         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-08 22:44 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Rex Feany

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 08/10/2009 23:04:03:
>
> On Thu, 2009-10-08 at 15:24 +0200, Joakim Tjernlund wrote:
>
> > +#define _PAGE_RW   0x0400   /* lsb PP bits, inverted in HW */
> > +#define _PAGE_USER   0x0800   /* msb PP bits */
> >
> > +   /* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
> > +   rlwimi.   r10, r10, 27, 31, 31
> > +   beq-   cr0, 2f /* Can be removed, costs a ITLB Err */
>
> Did you mean

(counting bits) ... No, it should be >> 5

>
> +    /* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>4) */
> +   rlwimi.   r10, r10, 28, 30, 31
> +   beq-   cr0, 2f /* Can be removed, costs a ITLB Err */
>
> Which would still be incorrect. You want -both- to be set,
> and your code will move on if -any- is set. Might want to add
> a ~ of the whole thing maybe and invert the condition ?

I want:
  if (!accessed)
     present = 0;

accessed == 1 and present = 0 is impossible, right?
So basically just copy over accessed to present and
linux mm set both when trapping to C.


>
> I find it easier to just do li rX, requested_bits, and then, andc.
> rscratch, rX, rPTE
>
> > +#if 0    /* Dont' bother with PP lsb, bit 21 for now */
> > +   /* r10 = (r10 & ~0x0400) | ((r10 & _PAGE_EXEC) << 7) */
> > +   rlwimi   r10, r10, 7, 21, 21 /* Set _PAGE_EXEC << 7 */
> > +#endif
>
> I don't get that one. Don't bother with _PAGE_EXEC, there's no
> control of execute permission that works on 8xx. It's #if 0 anyway.

What about the execute perms in Level 2 descriptor, page 247?

>
> You still need to massage the PP bits into place. I don't see that
> happening.

Not at the moment, later.

>
> As it is, your PTE contains for bit 20 and 21, which translates to:
>
>    PTE:                 Translates to PP bits:
> RW: 0   USER: 0          00  supervisor RW (ok)
> RW: 0   USER: 1          01  supervisor RW user RO (WRONG)
> RW: 1   USER: 0          10  supervisor RW user RW (WRONG)
> RW: 1   USER: 1          11  supervisor RO user RO (WRONG)

You got USER and RW swapped and the table is different
for exec.

>
> I would suggest you do the stuff I suggested initially with RW and USER
> being an "index" into a pre-cooked immediate value with all the encodings
> which also gives you the extended encoding for supervisor RO for free.
>
> > +   /* Need to know if load/store -> force a TLB Error
> > +    * by copying ACCESSED to PRESENT.
> > +   */
> > +   /* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
> > +   rlwimi   r10, r10, 27, 31, 31
>
> That doesn't sound right, just like ITLB... you need to AND those two
> bits in a way or another, or basically test that they are both set
> (or neither is set)

Same here as for ITLB.

>
> > +#if 0 /* Not yet */
> > +   /* Honour kernel RO, User NA */
> > +   andi.   r11, r10, _PAGE_USER | _PAGE_RW
> > +   bne-   cr0, 5f
> > +   ori   r10,r10, 0x200 /* Extended encoding, bit 22 */
> >  #endif
> > -   mfspr   r11, SPRN_MD_TWC   /* get the pte address again */
> > -   stw   r10, 0(r11)
> > +5:   xori   r10, r10, _PAGE_RW  /* invert RW bit */
>
> Well, you are missing that xori from the ITLB miss I think. Also, that

Nope, no xori needed for exec perms

> changes the table above to:
>
>    PTE:                 Translates to PP bits:
> RW: 0   USER: 0          10  supervisor RW user RW (WRONG)
> RW: 0   USER: 1          11  supervisor RO user RO (ok)
> RW: 1   USER: 0          00  supervisor RW (ok)
> RW: 1   USER: 1          01  supervisor RW user RO (WRONG)
>
> So it's still not right :-)

User and RW swapped here too, as I read the table.
I don't think user space would boot if I got it wrong.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] 8xx MMU fixes
  2009-10-08 13:24 [PATCH 0/6] 8xx MMU fixes Joakim Tjernlund
  2009-10-08 13:24 ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Joakim Tjernlund
@ 2009-10-09  0:15 ` Rex Feany
  2009-10-09  6:00   ` Joakim Tjernlund
  1 sibling, 1 reply; 18+ messages in thread
From: Rex Feany @ 2009-10-09  0:15 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev

Thus spake Joakim Tjernlund (Joakim.Tjernlund@transmode.se):

> So here we go again. This time I am
> fairly confindent I got most things correct :)
> Also manged to use even less instructions in the
> TLB Miss handlers.
> 
> Scott and Rex, forget previous versions and
> try this one out.

This patch set is better, userspace seems more stable but I am still
seeing some odd problems. This is from straceing mount, which displays
nothing even though /proc/mounts shows everything mounted properly:

open("/proc/mounts", O_RDONLY)          = 3
fstat64(0x3, 0x7fe7e2a8)                = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x3001f000
read(3, 0x3001f000, 1024)               = -1 EFAULT (Bad address)
exit_group(0)                           = ?

it looks like the memory allocated using mmap is bad?

take care!
/rex.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
  2009-10-08 22:44       ` Joakim Tjernlund
@ 2009-10-09  0:53         ` Benjamin Herrenschmidt
  2009-10-09  6:16           ` Joakim Tjernlund
  0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2009-10-09  0:53 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev, Rex Feany

On Fri, 2009-10-09 at 00:44 +0200, Joakim Tjernlund wrote:

> accessed == 1 and present = 0 is impossible, right?
> So basically just copy over accessed to present and
> linux mm set both when trapping to C.

No, when present = 0, then the rest of the PTE can contain unrelated
things, you can't trust ACCESSED.

> What about the execute perms in Level 2 descriptor, page 247?

Not useful, not fine grained enough.

> > You still need to massage the PP bits into place. I don't see that
> > happening.
> 
> Not at the moment, later.
> 
> >
> > As it is, your PTE contains for bit 20 and 21, which translates to:
> >
> >    PTE:                 Translates to PP bits:
> > RW: 0   USER: 0          00  supervisor RW (ok)
> > RW: 0   USER: 1          01  supervisor RW user RO (WRONG)
> > RW: 1   USER: 0          10  supervisor RW user RW (WRONG)
> > RW: 1   USER: 1          11  supervisor RO user RO (WRONG)
> 
> You got USER and RW swapped and the table is different
> for exec.

Hrm, let me see... yes. You are right, I mixed RW and USER. However,
I don't think the PP bits change do they ? IE. Basically, Read == Exec
at the page level. So the table isn't really different between I and D.

However, indeed, since you don't have a unified TLB, the case can be
made that we can ignore R vs. W in the iTLB case. In which case, you get
for iTLB:


    PTE:                 Translates to PP bits:
 RW: 0   USER: 0          00  supervisor X only (ok)
 RW: 0   USER: 1          10  supervisor X user X (ok)
 RW: 1   USER: 0          01  supervisor X user X (WRONG)
 RW: 1   USER: 1          11  supervisor X user X (ok)

So a page with _PAGE_RW and not _PAGE_USER would still be executable
from user... oops :-)

I think what you want for iTLB is just basically have a base of 00
and or-in _PAGE_USER only (ie, keep _PAGE_RW clear with a rlwinm)
so that you basically get supervisor X only if _PAGE_USER is 0 and
both X if _PAGE_USER is 1

For the dTLB, the table becomes (including your inversion of _PAGE_RW)

    PTE:                 Translates to PP bits: 
 RW: 0   USER: 0          01  supervisor RW user RO (WRONG)
 RW: 0   USER: 1          11  supervisor RO user RO (ok)
 RW: 1   USER: 0          00  supervisor RW only (ok)
 RW: 1   USER: 1          10  supervisor RW user RW (ok)

So it's -almost- right :-) You still got the RW:0 USER:0 case wrong,
ie a read-only kernel page would be user readable.

You can work around that by never setting kernel pages read-only (which
we do mostly), but in the grand scheme of things, my trick I proposed
initially would sort it out all including support for kernel RO :-)

In any case, the above, while wrong, wouldn't cause crashes or issues
for well behaved userspace so it's a step forward.

> Same here as for ITLB.

And still not right :-) ie. you cannot rely on the value of
_PAGE_ACCESSED if _PAGE_PRESENT is not set.

> Nope, no xori needed for exec perms

Right, thanks to having a split TLB, but you still need to mask
out one of the bits afaik.

> I don't think user space would boot if I got it wrong.

True. I think it's more correct than I initially thought but still
subtely wrong :-)

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] 8xx MMU fixes
  2009-10-09  0:15 ` [PATCH 0/6] 8xx MMU fixes Rex Feany
@ 2009-10-09  6:00   ` Joakim Tjernlund
  2009-10-09  6:46     ` Rex Feany
  0 siblings, 1 reply; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-09  6:00 UTC (permalink / raw)
  To: Rex Feany; +Cc: Scott Wood, linuxppc-dev

Rex Feany <RFeany@mrv.com> wrote on 09/10/2009 02:15:27:
>
> Thus spake Joakim Tjernlund (Joakim.Tjernlund@transmode.se):
>
> > So here we go again. This time I am
> > fairly confindent I got most things correct :)
> > Also manged to use even less instructions in the
> > TLB Miss handlers.
> >
> > Scott and Rex, forget previous versions and
> > try this one out.
>
> This patch set is better, userspace seems more stable but I am still
> seeing some odd problems. This is from straceing mount, which displays
> nothing even though /proc/mounts shows everything mounted properly:
>
> open("/proc/mounts", O_RDONLY)          = 3
> fstat64(0x3, 0x7fe7e2a8)                = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x3001f000
> read(3, 0x3001f000, 1024)               = -1 EFAULT (Bad address)
> exit_group(0)                           = ?
>
> it looks like the memory allocated using mmap is bad?

Try making the tlbil_va in fault.c unconditional, just to make sure
there isn't any old TLBs  around.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
  2009-10-09  0:53         ` Benjamin Herrenschmidt
@ 2009-10-09  6:16           ` Joakim Tjernlund
  2009-10-09  6:30             ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-09  6:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Rex Feany

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 09/10/2009 02:53:31:
>
> Subject:
>
> Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
>
> On Fri, 2009-10-09 at 00:44 +0200, Joakim Tjernlund wrote:
>
> > accessed == 1 and present = 0 is impossible, right?
> > So basically just copy over accessed to present and
> > linux mm set both when trapping to C.
>
> No, when present = 0, then the rest of the PTE can contain unrelated
> things, you can't trust ACCESSED.

Ah, OK.

>
> > What about the execute perms in Level 2 descriptor, page 247?
>
> Not useful, not fine grained enough.
>
> > > You still need to massage the PP bits into place. I don't see that
> > > happening.
> >
> > Not at the moment, later.
> >
> > >
> > > As it is, your PTE contains for bit 20 and 21, which translates to:
> > >
> > >    PTE:                 Translates to PP bits:
> > > RW: 0   USER: 0          00  supervisor RW (ok)
> > > RW: 0   USER: 1          01  supervisor RW user RO (WRONG)
> > > RW: 1   USER: 0          10  supervisor RW user RW (WRONG)
> > > RW: 1   USER: 1          11  supervisor RO user RO (WRONG)
> >
> > You got USER and RW swapped and the table is different
> > for exec.
>
> Hrm, let me see... yes. You are right, I mixed RW and USER. However,
> I don't think the PP bits change do they ? IE. Basically, Read == Exec
> at the page level. So the table isn't really different between I and D.
>
> However, indeed, since you don't have a unified TLB, the case can be
> made that we can ignore R vs. W in the iTLB case. In which case, you get
> for iTLB:
>
>
>     PTE:                 Translates to PP bits:
>  RW: 0   USER: 0          00  supervisor X only (ok)
>  RW: 0   USER: 1          10  supervisor X user X (ok)
>  RW: 1   USER: 0          01  supervisor X user X (WRONG)
>  RW: 1   USER: 1          11  supervisor X user X (ok)
>
> So a page with _PAGE_RW and not _PAGE_USER would still be executable
> from user... oops :-)

Yes, it is not perfect, but should work for now.

>
> I think what you want for iTLB is just basically have a base of 00
> and or-in _PAGE_USER only (ie, keep _PAGE_RW clear with a rlwinm)
> so that you basically get supervisor X only if _PAGE_USER is 0 and
> both X if _PAGE_USER is 1
>
> For the dTLB, the table becomes (including your inversion of _PAGE_RW)
>
>     PTE:                 Translates to PP bits:
>  RW: 0   USER: 0          01  supervisor RW user RO (WRONG)
>  RW: 0   USER: 1          11  supervisor RO user RO (ok)
>  RW: 1   USER: 0          00  supervisor RW only (ok)
>  RW: 1   USER: 1          10  supervisor RW user RW (ok)
>
> So it's -almost- right :-) You still got the RW:0 USER:0 case wrong,
> ie a read-only kernel page would be user readable.

Which will be fixed once I activate:
#if 0 /* Not yet */
	/* Honour kernel RO, User NA */
	andi.	r11, r10, _PAGE_USER | _PAGE_RW
	bne-	cr0, 5f
	ori	r10,r10, 0x200 /* Extended encoding, bit 22 */
#endif

>
> You can work around that by never setting kernel pages read-only (which
> we do mostly), but in the grand scheme of things, my trick I proposed
> initially would sort it out all including support for kernel RO :-)

Not convinced that your trick will be a win. The other
bits will need to move around too. Maybe I misunderstand
something?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.
  2009-10-09  6:16           ` Joakim Tjernlund
@ 2009-10-09  6:30             ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2009-10-09  6:30 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev, Rex Feany

On Fri, 2009-10-09 at 08:16 +0200, Joakim Tjernlund wrote:

> Which will be fixed once I activate:
> #if 0 /* Not yet */
> 	/* Honour kernel RO, User NA */
> 	andi.	r11, r10, _PAGE_USER | _PAGE_RW
> 	bne-	cr0, 5f
> 	ori	r10,r10, 0x200 /* Extended encoding, bit 22 */
> #endif

Which will be more code including a conditional than my proposed
trick :-) I'll try to write the asm for you later.

> Not convinced that your trick will be a win. The other
> bits will need to move around too. Maybe I misunderstand
> something?

I'll write some code to show you what I have in mind later.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] 8xx MMU fixes
  2009-10-09  6:00   ` Joakim Tjernlund
@ 2009-10-09  6:46     ` Rex Feany
  2009-10-09 11:04       ` Joakim Tjernlund
  2009-10-09 12:30       ` Joakim Tjernlund
  0 siblings, 2 replies; 18+ messages in thread
From: Rex Feany @ 2009-10-09  6:46 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev

Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):

> Rex Feany <RFeany@mrv.com> wrote on 09/10/2009 02:15:27:

> > open("/proc/mounts", O_RDONLY)          = 3
> > fstat64(0x3, 0x7fe7e2a8)                = 0
> > mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x3001f000
> > read(3, 0x3001f000, 1024)               = -1 EFAULT (Bad address)
> > exit_group(0)                           = ?
> 
> Try making the tlbil_va in fault.c unconditional, just to make sure
> there isn't any old TLBs  around.

didn't make a difference

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] 8xx MMU fixes
  2009-10-09  6:46     ` Rex Feany
@ 2009-10-09 11:04       ` Joakim Tjernlund
  2009-10-09 12:30       ` Joakim Tjernlund
  1 sibling, 0 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-09 11:04 UTC (permalink / raw)
  To: Rex Feany; +Cc: Scott Wood, linuxppc-dev



Rex Feany <RFeany@mrv.com> wrote on 09/10/2009 08:46:49:
>
> Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
>
> > Rex Feany <RFeany@mrv.com> wrote on 09/10/2009 02:15:27:
>
> > > open("/proc/mounts", O_RDONLY)          = 3
> > > fstat64(0x3, 0x7fe7e2a8)                = 0
> > > mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =0x3001f000
> > > read(3, 0x3001f000, 1024)               = -1 EFAULT (Bad address)
> > > exit_group(0)                           = ?
> >
> > Try making the tlbil_va in fault.c unconditional, just to make sure
> > there isn't any old TLBs  around.
>
> didn't make a difference

OK, so how about:
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 6541855..f4b5dca 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -339,9 +339,9 @@ InstructionTLBMiss:
 	mfspr	r11, SPRN_MD_TWC	/* ....and get the pte address */
 	lwz	r10, 0(r11)	/* Get the pte */

-	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
-	rlwimi.	r10, r10, 27, 31, 31
-	beq-	cr0, 2f /* Can be removed, costs a ITLB Err */
+	andi.	r11, r10, _PAGE_ACCESSED | _PAGE_PRESENT
+	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
+	bne-	cr0, 2f

 #if 0 	/* Dont' bother with PP lsb, bit 21 for now */
 	/* r10 = (r10 & ~0x0400) | ((r10 & _PAGE_EXEC) << 7) */
@@ -429,9 +429,11 @@ DataStoreTLBMiss:
 	/* Need to know if load/store -> force a TLB Error
 	 * by copying ACCESSED to PRESENT.
 	*/
-	/* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
-	rlwimi	r10, r10, 27, 31, 31
-
+	andi.	r11, r10, _PAGE_ACCESSED | _PAGE_PRESENT
+	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
+	beq+	cr0, 6f
+	rlwinm	r10, r10, 0, 0, 30 /* Clear _PAGE_PRESENT */
+6:
 #if 0 /* Not yet */
 	/* Honour kernel RO, User NA */
 	andi.	r11, r10, _PAGE_USER | _PAGE_RW
@@ -492,7 +494,7 @@ DataTLBError:
 	cmpwi	cr0, r10, 0x00f0
 	beq-	FixDAR	/* must be a buggy dcbX, icbi insn. */
 DARFix:	/* Return from dcbx instruction bug workaround, r10 holds value of DAR */
-
+	b	2f /* Do DIRTY in C */
 	mfspr	r11, SPRN_DSISR
 	andis.	r11, r11, 0x4800	/* !translation or protection */
 	bne	2f	/* branch if either is set */

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] 8xx MMU fixes
  2009-10-09  6:46     ` Rex Feany
  2009-10-09 11:04       ` Joakim Tjernlund
@ 2009-10-09 12:30       ` Joakim Tjernlund
  1 sibling, 0 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2009-10-09 12:30 UTC (permalink / raw)
  To: Rex Feany; +Cc: Scott Wood, linuxppc-dev

Rex Feany <RFeany@mrv.com> wrote on 09/10/2009 08:46:49:
>
> Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
>
> > Rex Feany <RFeany@mrv.com> wrote on 09/10/2009 02:15:27:
>
> > > open("/proc/mounts", O_RDONLY)          = 3
> > > fstat64(0x3, 0x7fe7e2a8)                = 0
> > > mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =0x3001f000
> > > read(3, 0x3001f000, 1024)               = -1 EFAULT (Bad address)
> > > exit_group(0)                           = ?
> >
> > Try making the tlbil_va in fault.c unconditional, just to make sure
> > there isn't any old TLBs  around.
>
> didn't make a difference

Perhaps you are suffering from a buggy dcbst insn? I tested it
on a RO mapping and it SEGVs. Clearing the store bit manually
at least fixes the SEGVs.

Here is a patch for that.

     Jocke

>From 07dbca0cf9dc13cf0fbccf54d577e3bc1c5dfdf1 Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Fri, 9 Oct 2009 14:18:21 +0200
Subject: [PATCH] 8xx: dcbst sets store bit in DTLB error, workaround.

dcbst should not set the store bit(bit 6, DSISR) when
trapping into a DTLB Error. Clear this bit while doing
the dcbX missing DAR workaround.
---
 arch/powerpc/kernel/head_8xx.S |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 292bd87..7b31feb 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -630,6 +630,30 @@ FixDAR:	/* Entry point for dcbx workaround. */
 	tophys  (r11, r10)
 	beq-	139b		/* Branch if user space address */
 140:	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	DARfix		/* 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
--
1.6.4.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2009-10-09 12:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-08 13:24 [PATCH 0/6] 8xx MMU fixes Joakim Tjernlund
2009-10-08 13:24 ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Joakim Tjernlund
2009-10-08 13:24   ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Joakim Tjernlund
2009-10-08 13:24     ` [PATCH 3/6] 8xx: invalidate non present TLBs Joakim Tjernlund
2009-10-08 13:24       ` [PATCH 4/6] 8xx: Tag DAR with 0x00f0 to catch buggy instructions Joakim Tjernlund
2009-10-08 13:24         ` [PATCH 5/6] 8xx: Fixup DAR from buggy dcbX instructions Joakim Tjernlund
2009-10-08 13:24           ` [PATCH 6/6] 8xx: start using dcbX instructions in various copy routines Joakim Tjernlund
2009-10-08 21:04     ` [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects Benjamin Herrenschmidt
2009-10-08 22:44       ` Joakim Tjernlund
2009-10-09  0:53         ` Benjamin Herrenschmidt
2009-10-09  6:16           ` Joakim Tjernlund
2009-10-09  6:30             ` Benjamin Herrenschmidt
2009-10-08 20:48   ` [PATCH 1/6] 8xx: DTLB Error must check for more errors Benjamin Herrenschmidt
2009-10-09  0:15 ` [PATCH 0/6] 8xx MMU fixes Rex Feany
2009-10-09  6:00   ` Joakim Tjernlund
2009-10-09  6:46     ` Rex Feany
2009-10-09 11:04       ` Joakim Tjernlund
2009-10-09 12:30       ` Joakim Tjernlund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).