[PATCH 2.6.14] mm: 8xx MM fix for

[Joakim Tjernlund]

> 
> On Mon, Nov 07, 2005 at 07:37:45PM +0100, Joakim Tjernlund wrote:
> >  > 
> > > On Mon, Nov 07, 2005 at 07:14:15PM +0100, Joakim Tjernlund wrote:
> > > > > -----Original Message-----
> > > > > From: Tom Rini [mailto:trini at kernel.crashing.org] 
> > > > > Sent: 07 November 2005 16:52
> > > > > To: Marcelo Tosatti
> > > > > Cc: Joakim Tjernlund; Pantelis Antoniou; Dan Malek; 
> > > > > linuxppc-embedded at ozlabs.org; gtolstolytkin at ru.mvista.com
> > > > > Subject: Re: [PATCH 2.6.14] mm: 8xx MM fix for
> > > > > 
> > > > > On Mon, Nov 07, 2005 at 08:16:18AM -0200, Marcelo 
> Tosatti wrote:
> > > > > > Joakim!
> > > > > > 
> > > > > > On Mon, Nov 07, 2005 at 03:32:52PM +0100, Joakim 
> > > Tjernlund wrote:
> > > > > > > Hi Marcelo
> > > > > > > 
> > > > > > > [SNIP] 
> > > > > > > > The root of the problem are the changes against 
> the 8xx TLB 
> > > > > > > > handlers introduced
> > > > > > > > during v2.6. What happens is the TLBMiss 
> handlers load the 
> > > > > > > > zeroed pte into
> > > > > > > > the TLB, causing the TLBError handler to be 
> invoked (thats 
> > > > > > > > two TLB faults per 
> > > > > > > > pagefault), which then jumps to the generic MM code to 
> > > > > setup the pte.
> > > > > > > > 
> > > > > > > > The bug is that the zeroed TLB is not invalidated (the 
> > > > > same reason
> > > > > > > > for the "dcbst" misbehaviour), resulting in infinite 
> > > > > TLBError faults.
> > > > > > > > 
> > > > > > > > Dan, I wonder why we just don't go back to v2.4 
> behaviour.
> > > > > > > 
> > > > > > > This is one reason why it is the way it is:
> > > > > > > 
> > > > > 
> > > 
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-January/016382.html
> > > > > > > This details are little fuzzy ATM, but I think the 
> > > reason for the
> > > > > > > current
> > > > > > > impl. was only that it was less intrusive to impl.
> > > > > > 
> > > > > > Ah, I see. I wonder if the bug is processor specific: we 
> > > > > don't have such
> > > > > > changes in our v2.4 tree and never experienced such problem.
> > > > > > 
> > > > > > It should be pretty easy to hit it right? (instruction 
> > > > > pagefaults should
> > > > > > fail).
> > > > > > 
> > > > > > Grigori, Tom, can you enlight us about the issue on the URL 
> > > > > above. How
> > > > > > can it be triggered?
> > > > > 
> > > > > So after looking at the code in 2.6.14 and current git, I 
> > > think the
> > > > > above URL isn't relevant, unless there was a change I 
> > > missed (which
> > > > > could totally be possible) that reverted the patch there and 
> > > > > fixed that
> > > > > issue in a different manner.  But since I didn't figure that 
> > > > > out until I
> > > > > had finished researching it again:
> > > > 
> > > > I wasn't clear enough. What I meant was that the above 
> patch made me
> > > > think and
> > > > the result was that I came up with a simpler fix, the "two 
> > > exception"
> > > > fix that
> > > > is in current kernels. See
> > > > 
> > >
http://linux.bkbits.net:8080/linux-2.6/diffs/arch/ppc/kernel/head_8xx.S@
> > > > 
> > > 1.19?nav=index.html|src/.|src/arch|src/arch/ppc|src/arch/ppc/k
> > > ernel|hist
> > > > /arch/ppc/kernel/head_8xx.S
> > > > It appears this fix has some other issues :(
> > > > 
> > > > How do the other ppc arches do? I am guessing that they 
> don't double
> > > > fault, but bails
> > > > out to do_page_fault from the TLB Miss handler, like 
> 8xx used to do.
> > > 
> > > Assuming Dan doesn't come up with a more simple & better 
> fix, maybe we
> > > should go back to the original patch I made?
> > 
> > That was what I was thinking too(or some variation of your patch)
> > I wonder if that would solve the misbehaving dcbst problem 
> Marcelo found
> > some time ago too?
> 
> Hi Joakim,
> 
> Yes, it would fix the "dcbst" issue. That problem was triggered by a
> zeroed TLB entry.
> 
> In practice it seems that the "three exception" approach does 
> not impose
> a significant overhead in comparison with the "two exception" version
> (as can be seen by the results of the latency tests).
> 
> Anyway, if decided upon, the "two exception" version (no zeroed TLB
> entry state) needs the TLBMiss handler should to the present 
> bit as Dan
> mentioned.
> 
> I don't know what Dan is up to, he meant to be doing 
> significant changes.

Dan, any progress?

> 
> I'll be playing with TLB preloading next week... how's your 
> TLB handler
> shrinkage idea?

Well, here is a first step.
This patch will save 2 instructions in the hot path and removes the need
to save 
registers around address 0 in the ITLB Miss handler IFF you use pinned
TLBs and no modules.

What do you think?

 Jocke

--- head_8xx.S.org	Wed Nov 30 16:38:30 2005
+++ head_8xx.S	Wed Nov 30 18:19:16 2005
@@ -300,14 +300,17 @@
 #endif
 	DO_8xx_CPU6(0x3f80, r3)
 	mtspr	SPRN_M_TW, r10	/* Save a couple of working registers */
+#if !CONFIG_PIN_TLB || CONFIG_MODULES
 	mfcr	r10
 	stw	r10, 0(r0)
-	stw	r11, 4(r0)
+#endif
+	mtspr	SPRN_SPRG2, r11	/* Save a couple of working registers */
 	mfspr	r10, SPRN_SRR0	/* Get effective address of fault */
 	DO_8xx_CPU6(0x3780, r3)
 	mtspr	SPRN_MD_EPN, r10	/* Have to use MD_EPN for walk,
MI_EPN can't */
 	mfspr	r10, SPRN_M_TWB	/* Get level 1 table entry address */
 
+#if !CONFIG_PIN_TLB || CONFIG_MODULES
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
@@ -319,12 +322,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, don't try to find a pte */
-
+	ori	r11,r11,1	/* Set valid bit */
+	beq-	2f		/* If zero, don't try to find a pte */
+#else
+	lwz	r11, 0(r10)	/* Get the level 1 entry */
+	mfcr	r10		/* Save CR */
+	cmplwi	cr0,r11,0x0fff	/* Test page descriptor page address */
+	ori	r11,r11,1	/* Set valid bit */
+	bng-	3f		/* If zero, don't try to find a pte */
+	mtcr	r10		/* Restore CR */
+#endif
 	/* We have a pte table, so load the MI_TWC with the attributes
 	 * for this "segment."
 	 */
-	ori	r11,r11,1		/* Set valid bit */
 	DO_8xx_CPU6(0x2b80, r3)
 	mtspr	SPRN_MI_TWC, r11	/* Set segment attributes */
 	DO_8xx_CPU6(0x3b80, r3)
@@ -347,13 +357,19 @@
 	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */
 
 	mfspr	r10, SPRN_M_TW	/* Restore registers */
+#if !CONFIG_PIN_TLB || CONFIG_MODULES
 	lwz	r11, 0(r0)
 	mtcr	r11
-	lwz	r11, 4(r0)
+#endif
+	mfspr	r11, SPRN_SPRG2	/* Restore registers */
 #ifdef CONFIG_8xx_CPU6
 	lwz	r3, 8(r0)
 #endif
 	rfi
+#if !CONFIG_PIN_TLB || CONFIG_MODULES
+3:	mtcr	r10
+	ba	2b
+#endif
 
 	. = 0x1200
 DataStoreTLBMiss: