linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] Handle I-TLB Error and Miss separately on 8xx
@ 2005-01-12  7:53 Joakim Tjernlund
  2005-01-12 14:06 ` Tom Rini
  0 siblings, 1 reply; 21+ messages in thread
From: Joakim Tjernlund @ 2005-01-12  7:53 UTC (permalink / raw)
  To: Tom Rini, Linuxppc-Embedded@Ozlabs. Org

> As the code stands currently, there is a bug in the 2.4 and 2.6 handling
> of I-TLB Miss and Error exceptions on 8xx.  The problem is that since we
> treat both of them as the same exception when we hit do_page_fault,
> there is a case where we can incorrectly find that a protection fault
> has occured, when it hasn't.  This is because we check bit 4 of SRR1 in
> both cases, but in the case of an I-TLB Miss, this bit is always set,
> and it only indicates a protection fault on an I-TLB Error.

Patch looks good to me, but I want to ask when this error
can be triggered in practice?

I have never seen it happen and it makes me wonder if the test
for a null pte in the I-TLB Miss handler is needed?

In linuxppc-2.4 there is a special case for pinned tlbs were
one could remove 4 instructions if the test for null ptes is removed.

I belive SPRG2 is free in 2.6 and if combined with the special case for pinned
tlbs in linuxppc-2.4 one can remove all memory references used for temporary
storage in the I-TLB Miss handler. That will save a cache line load&store.

 Jocke

^ permalink raw reply	[flat|nested] 21+ messages in thread
* [PATCH] Handle I-TLB Error and Miss separately on 8xx
@ 2005-01-07 16:22 Tom Rini
  0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2005-01-07 16:22 UTC (permalink / raw)
  To: linuxppc-embedded

As the code stands currently, there is a bug in the 2.4 and 2.6 handling
of I-TLB Miss and Error exceptions on 8xx.  The problem is that since we
treat both of them as the same exception when we hit do_page_fault,
there is a case where we can incorrectly find that a protection fault
has occured, when it hasn't.  This is because we check bit 4 of SRR1 in
both cases, but in the case of an I-TLB Miss, this bit is always set,
and it only indicates a protection fault on an I-TLB Error.

Originally from Grigori Tolstolytkin <gtolstolytkin@ru.mvista.com>.
Signed-off-by: Tom Rini <trini@kernel.crashing.org>

Patch vs 2.4-current:
--- 1.19/arch/ppc/kernel/head_8xx.S	2003-10-27 12:31:25 -07:00
+++ edited/arch/ppc/kernel/head_8xx.S	2005-01-07 08:57:31 -07:00
@@ -501,10 +501,18 @@
 /* This is an instruction TLB error on the MPC8xx.  This could be due
  * to many reasons, such as executing guarded memory or illegal instruction
  * addresses.  There is nothing to do but handle a big time error fault.
+ * But we can't just jump from the InstructionAccess fault (0x400) as
+ * do_page_fault() needs to know.
  */
 	. = 0x1300
 InstructionTLBError:
-	b	InstructionAccess
+	EXCEPTION_PROLOG
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	mr	r4,r22
+	mr	r5,r23
+	li	r20,MSR_KERNEL
+	rlwimi	r20,r23,0,16,16		/* copy EE bit from saved MSR */
+	FINISH_EXCEPTION(do_page_fault)
 
 /* This is the data TLB error on the MPC8xx.  This could be due to
  * many reasons, including a dirty update to a pte.  We can catch that
--- 1.15/arch/ppc/mm/fault.c	2003-08-29 03:37:49 -07:00
+++ edited/arch/ppc/mm/fault.c	2005-01-07 08:59:25 -07:00
@@ -91,7 +91,8 @@
  * For 600- and 800-family processors, the error_code parameter is DSISR
  * for a data fault, SRR1 for an instruction fault. For 400-family processors
  * the error_code parameter is ESR for a data fault, 0 for an instruction
- * fault.
+ * fault.  On 800-family processors, we fudge an I-TLB Miss (0x1100) as
+ * being at 0x400 for space reasons.
  */
 void do_page_fault(struct pt_regs *regs, unsigned long address,
 		   unsigned long error_code)
@@ -111,7 +112,11 @@
 	 * bits we are interested in.  But there are some bits which
 	 * indicate errors in DSISR but can validly be set in SRR1.
 	 */
+#ifdef CONFIG_8xx
+	if (regs->trap == 0x400 || regs->trap == 0x1300)
+#else
 	if (regs->trap == 0x400)
+#endif
 		error_code &= 0x48200000;
 	else
 		is_write = error_code & 0x02000000;
@@ -204,8 +209,17 @@
 			goto bad_area;
 	/* a read */
 	} else {
-		/* protection fault */
+		/*
+		 * On non-8xx, a protection fault.  On 8xx, this bit is
+		 * always set on I-TLB Miss, but indicates a protection
+		 * fault on an I-TLB Error.  So we only check this bit
+		 * if we aren't an I-TLB Miss.
+		 */
+#ifdef CONFIG_8xx
+		if ((error_code & 0x08000000) && regs->trap != 0x400)
+#else
 		if (error_code & 0x08000000)
+#endif
 			goto bad_area;
 		if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
 			goto bad_area;

Patch vs 2.6-current:
--- 1.18/arch/ppc/kernel/head_8xx.S	2004-11-11 01:25:53 -07:00
+++ edited/arch/ppc/kernel/head_8xx.S	2005-01-07 09:13:05 -07:00
@@ -445,10 +445,15 @@
 /* This is an instruction TLB error on the MPC8xx.  This could be due
  * to many reasons, such as executing guarded memory or illegal instruction
  * addresses.  There is nothing to do but handle a big time error fault.
+ * But we can't just jump from the InstructionAccess fault (0x400) as
+ * do_page_fault() needs to know.
  */
 	. = 0x1300
 InstructionTLBError:
-	b	InstructionAccess
+	EXCEPTION_PROLOG
+	mr	r4,r12
+	mr	r5,r9
+	EXC_XFER_EE_LITE(0x1300, handle_page_fault)
 
 /* This is the data TLB error on the MPC8xx.  This could be due to
  * many reasons, including a dirty update to a pte.  We can catch that
--- 1.21/arch/ppc/mm/fault.c	2004-07-26 14:43:22 -07:00
+++ edited/arch/ppc/mm/fault.c	2005-01-07 09:11:44 -07:00
@@ -90,7 +90,8 @@
  * For 600- and 800-family processors, the error_code parameter is DSISR
  * for a data fault, SRR1 for an instruction fault. For 400-family processors
  * the error_code parameter is ESR for a data fault, 0 for an instruction
- * fault.
+ * fault.  On 800-family processors, we fudge an I-TLB Miss (0x1100) as
+ * being at 0x400 for space reasons.
  */
 int do_page_fault(struct pt_regs *regs, unsigned long address,
 		  unsigned long error_code)
@@ -110,7 +111,11 @@
 	 * bits we are interested in.  But there are some bits which
 	 * indicate errors in DSISR but can validly be set in SRR1.
 	 */
-	if (TRAP(regs) == 0x400)
+#ifdef CONFIG_8xx
+	if (TRAP(regs) == 0x400 || TRAP(regs) == 0x1300)
+#else
+ 	if (TRAP(regs) == 0x400)
+#endif
 		error_code &= 0x48200000;
 	else
 		is_write = error_code & 0x02000000;
@@ -235,8 +240,17 @@
 #endif
 	/* a read */
 	} else {
-		/* protection fault */
+		/*
+		 * On non-8xx, a protection fault.  On 8xx, this bit is
+		 * always set on I-TLB Miss, but indicates a protection
+		 * fault on an I-TLB Error.  So we only check this bit
+		 * if we aren't an I-TLB Miss.
+		 */
+#ifdef CONFIG_8xx
+		if ((error_code & 0x08000000) && regs->trap != 0x400)
+#else
 		if (error_code & 0x08000000)
+#endif
 			goto bad_area;
 		if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
 			goto bad_area;

-- 
Tom Rini
http://gate.crashing.org/~trini/

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

end of thread, other threads:[~2005-01-19 19:58 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-01-12  7:53 [PATCH] Handle I-TLB Error and Miss separately on 8xx Joakim Tjernlund
2005-01-12 14:06 ` Tom Rini
2005-01-12 14:17   ` Joakim Tjernlund
2005-01-12 15:15     ` Tom Rini
2005-01-13 15:16       ` Tom Rini
2005-01-14 14:03         ` Joakim Tjernlund
2005-01-14 17:38           ` Joakim Tjernlund
2005-01-14 17:47             ` Tom Rini
2005-01-14 17:56               ` Joakim Tjernlund
2005-01-14 18:05                 ` Tom Rini
2005-01-14 19:51                   ` Joakim Tjernlund
2005-01-18 20:09                     ` Tom Rini
2005-01-18 22:02                       ` Joakim Tjernlund
2005-01-19 17:25                         ` Dan Malek
2005-01-19 18:06                           ` Joakim Tjernlund
2005-01-19 18:37                             ` Dan Malek
2005-01-19 19:58                               ` Joakim Tjernlund
2005-01-19  0:30                       ` Joakim Tjernlund
2005-01-19 17:28                         ` Dan Malek
2005-01-19 18:12                           ` Joakim Tjernlund
  -- strict thread matches above, loose matches on Subject: below --
2005-01-07 16:22 Tom Rini

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).