Patchwork [v2] powerpc 8xx: Fixing issue with CONFIG_PIN_TLB

login
register
mail settings
Submitter LEROY Christophe
Date Sept. 12, 2013, 6:25 p.m.
Message ID <201309121825.r8CIPrkI005690@localhost.localdomain>
Download mbox | patch
Permalink /patch/274583/
State Superseded
Headers show

Comments

LEROY Christophe - Sept. 12, 2013, 6:25 p.m.
This is a reorganisation of the setup of the TLB at kernel startup, in order
to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866
and MPC885 reference manuals.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Scott Wood - Sept. 12, 2013, 6:44 p.m.
On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote:
> This is a reorganisation of the setup of the TLB at kernel startup, in order
> to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866
> and MPC885 reference manuals.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 
> diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S
> --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S	2013-09-02 22:46:10.000000000 +0200
> +++ linux-3.11/arch/powerpc/kernel/head_8xx.S	2013-09-09 11:28:54.000000000 +0200
> @@ -785,27 +785,24 @@
>   * these mappings is mapped by page tables.
>   */
>  initial_mmu:
> -	tlbia			/* Invalidate all TLB entries */
> -/* Always pin the first 8 MB ITLB to prevent ITLB
> -   misses while mucking around with SRR0/SRR1 in asm
> -*/
> -	lis	r8, MI_RSV4I@h
> -	ori	r8, r8, 0x1c00
> -
> +	lis	r8, MI_RESETVAL@h
>  	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
>  
> -#ifdef CONFIG_PIN_TLB
> -	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
> -	ori	r10, r10, 0x1c00
> -	mr	r8, r10
> -#else
>  	lis	r10, MD_RESETVAL@h
> -#endif
>  #ifndef CONFIG_8xx_COPYBACK
>  	oris	r10, r10, MD_WTDEF@h
>  #endif
>  	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
>  
> +	tlbia			/* Invalidate all TLB entries */

Is this change to make sure we invalidate everything even if the
bootloader set RSV4I?

> +	ori	r8, r8, 0x1c00
> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> +#ifdef CONFIG_PIN_TLB
> +	ori	r10, r10, 0x1c00
> +	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
> +#endif

Still 0x1c00?

>  	/* Now map the lower 8 Meg into the TLBs.  For this quick hack,
>  	 * we can load the instruction and data TLB registers with the
>  	 * same values.
> @@ -825,6 +822,12 @@
>  	mtspr	SPRN_MI_AP, r8
>  	mtspr	SPRN_MD_AP, r8
>  
> +	/* Always pin the first 8 MB ITLB to prevent ITLB
> +	 * misses while mucking around with SRR0/SRR1 in asm
> +	 */
> +	lis	r8, (MI_RSV4I | MI_RESETVAL)@h
> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */

Entry 0 is not pinnable.

-Scott
LEROY Christophe - Sept. 13, 2013, 5:04 a.m.
Le 12/09/2013 20:44, Scott Wood a écrit :
> On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote:
>> This is a reorganisation of the setup of the TLB at kernel startup, in order
>> to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866
>> and MPC885 reference manuals.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S
>> --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S	2013-09-02 22:46:10.000000000 +0200
>> +++ linux-3.11/arch/powerpc/kernel/head_8xx.S	2013-09-09 11:28:54.000000000 +0200
>> @@ -785,27 +785,24 @@
>>    * these mappings is mapped by page tables.
>>    */
>>   initial_mmu:
>> -	tlbia			/* Invalidate all TLB entries */
>> -/* Always pin the first 8 MB ITLB to prevent ITLB
>> -   misses while mucking around with SRR0/SRR1 in asm
>> -*/
>> -	lis	r8, MI_RSV4I@h
>> -	ori	r8, r8, 0x1c00
>> -
>> +	lis	r8, MI_RESETVAL@h
>>   	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
>>   
>> -#ifdef CONFIG_PIN_TLB
>> -	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
>> -	ori	r10, r10, 0x1c00
>> -	mr	r8, r10
>> -#else
>>   	lis	r10, MD_RESETVAL@h
>> -#endif
>>   #ifndef CONFIG_8xx_COPYBACK
>>   	oris	r10, r10, MD_WTDEF@h
>>   #endif
>>   	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
>>   
>> +	tlbia			/* Invalidate all TLB entries */
> Is this change to make sure we invalidate everything even if the
> bootloader set RSV4I?
Most probably. It is step 2 of the process defined in MPC866 and MPC885 
Reference Manuals:

§8.10.3 Loading Locked TLB Entries:
The process of loading a single reserved entry in the TLB is as follows:
1. Disable the TLB by clearing MSR[IR] or MSR[DR] as needed.
2. Clear MI_CTR[RSV4I] (MD_CTR[RSV4D]).
3. Invalidate the EA of the reserved page by using tlbia or tlbie.
4. Set MI_CTR[ITLB_INDX] (MD_CTR[DTLB_INDX]) to the appropriate value 
(between 27 and 31).
5. Load Mx_EPN with the effective page number, the ASID of the reserved 
page, and set EV.
6. Run software tablewalk code to load the appropriate entry into the 
translation lookaside buffer. See Section 8.10.1.1, “Translation Reload 
Examples.”
7. Repeat steps 4–6 to load other TLB entries.
8. Set MI_CTR[RSV4I] (MD_CTR[RSV4D])
>
>> +	ori	r8, r8, 0x1c00
>> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
>> +#ifdef CONFIG_PIN_TLB
>> +	ori	r10, r10, 0x1c00
>> +	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
>> +#endif
> Still 0x1c00?
Yes, I kept the same entries in order to limit modifications:
* 28 = First 8Mbytes page
* 29 = IMMR
* 30 = Second 8Mbytes page
* 31 = Third 8Mbytes page
>
>>   	/* Now map the lower 8 Meg into the TLBs.  For this quick hack,
>>   	 * we can load the instruction and data TLB registers with the
>>   	 * same values.
>> @@ -825,6 +822,12 @@
>>   	mtspr	SPRN_MI_AP, r8
>>   	mtspr	SPRN_MD_AP, r8
>>   
>> +	/* Always pin the first 8 MB ITLB to prevent ITLB
>> +	 * misses while mucking around with SRR0/SRR1 in asm
>> +	 */
>> +	lis	r8, (MI_RSV4I | MI_RESETVAL)@h
>> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> Entry 0 is not pinnable.
Here we are not trying to pin entry 0. We are at step 8, we are setting 
MI_RSV4I. At the same time, we set MD_CTR to 0 which is off the pinned 
range, to be sure that we won't overwrite one of the pinned entries.

The main difference compared to the previous implementation is that 
before, we were setting the RSV4I bit before loading the TLB entries. 
Now, as defined in the Reference Manuals, we are doing it at the end.

Christophe
Scott Wood - Sept. 16, 2013, 9:02 p.m.
On Fri, 2013-09-13 at 07:04 +0200, leroy christophe wrote:
> Le 12/09/2013 20:44, Scott Wood a écrit :
> > On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote:
> >> This is a reorganisation of the setup of the TLB at kernel startup, in order
> >> to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866
> >> and MPC885 reference manuals.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >>
> >> diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S
> >> --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S	2013-09-02 22:46:10.000000000 +0200
> >> +++ linux-3.11/arch/powerpc/kernel/head_8xx.S	2013-09-09 11:28:54.000000000 +0200
> >> @@ -785,27 +785,24 @@
> >>    * these mappings is mapped by page tables.
> >>    */
> >>   initial_mmu:
> >> -	tlbia			/* Invalidate all TLB entries */
> >> -/* Always pin the first 8 MB ITLB to prevent ITLB
> >> -   misses while mucking around with SRR0/SRR1 in asm
> >> -*/
> >> -	lis	r8, MI_RSV4I@h
> >> -	ori	r8, r8, 0x1c00
> >> -
> >> +	lis	r8, MI_RESETVAL@h
> >>   	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> >>   
> >> -#ifdef CONFIG_PIN_TLB
> >> -	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
> >> -	ori	r10, r10, 0x1c00
> >> -	mr	r8, r10
> >> -#else
> >>   	lis	r10, MD_RESETVAL@h
> >> -#endif
> >>   #ifndef CONFIG_8xx_COPYBACK
> >>   	oris	r10, r10, MD_WTDEF@h
> >>   #endif
> >>   	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
> >>   
> >> +	tlbia			/* Invalidate all TLB entries */
> > Is this change to make sure we invalidate everything even if the
> > bootloader set RSV4I?
> Most probably. It is step 2 of the process defined in MPC866 and MPC885 
> Reference Manuals:
> 
> §8.10.3 Loading Locked TLB Entries:
> The process of loading a single reserved entry in the TLB is as follows:

To minimize code churn we should just fix actual problems, rather than
shuffle things around to conform to a suggested sequence.  After all,
we're not just trying to load a single entry.

> >> +	ori	r8, r8, 0x1c00
> >> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> >> +#ifdef CONFIG_PIN_TLB
> >> +	ori	r10, r10, 0x1c00
> >> +	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
> >> +#endif
> > Still 0x1c00?
> Yes, I kept the same entries in order to limit modifications:
> * 28 = First 8Mbytes page
> * 29 = IMMR
> * 30 = Second 8Mbytes page
> * 31 = Third 8Mbytes page

If you actually want to program them in increasing order then it looks
like you're still missing a write to CTR between the last two 8M entries
-- thus you'll overwrite the IMMR with the last 8M entry.  That was the
same problem that v1 fixed -- did that change get lost accidentally?

The hardware wants to decrement; why fight it?

> >>   	/* Now map the lower 8 Meg into the TLBs.  For this quick hack,
> >>   	 * we can load the instruction and data TLB registers with the
> >>   	 * same values.
> >> @@ -825,6 +822,12 @@
> >>   	mtspr	SPRN_MI_AP, r8
> >>   	mtspr	SPRN_MD_AP, r8
> >>   
> >> +	/* Always pin the first 8 MB ITLB to prevent ITLB
> >> +	 * misses while mucking around with SRR0/SRR1 in asm
> >> +	 */
> >> +	lis	r8, (MI_RSV4I | MI_RESETVAL)@h
> >> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> > Entry 0 is not pinnable.
> Here we are not trying to pin entry 0.

Sorry, misread the patch.

> We are at step 8, we are setting 
> MI_RSV4I. At the same time, we set MD_CTR to 0 which is off the pinned 
> range, to be sure that we won't overwrite one of the pinned entries.
>
> The main difference compared to the previous implementation is that 
> before, we were setting the RSV4I bit before loading the TLB entries. 
> Now, as defined in the Reference Manuals, we are doing it at the end.

Have you seen any evidence that it matters?

-Scott
LEROY Christophe - Sept. 17, 2013, 4:40 p.m.
Le 16/09/2013 23:02, Scott Wood a écrit :
> On Fri, 2013-09-13 at 07:04 +0200, leroy christophe wrote:
>> Le 12/09/2013 20:44, Scott Wood a écrit :
>>> On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote:
>>>> This is a reorganisation of the setup of the TLB at kernel startup, in order
>>>> to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866
>>>> and MPC885 reference manuals.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>
>>>> diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S
>>>> --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S	2013-09-02 22:46:10.000000000 +0200
>>>> +++ linux-3.11/arch/powerpc/kernel/head_8xx.S	2013-09-09 11:28:54.000000000 +0200
>>>> @@ -785,27 +785,24 @@
>>>>     * these mappings is mapped by page tables.
>>>>     */
>>>>    initial_mmu:
>>>> -	tlbia			/* Invalidate all TLB entries */
>>>> -/* Always pin the first 8 MB ITLB to prevent ITLB
>>>> -   misses while mucking around with SRR0/SRR1 in asm
>>>> -*/
>>>> -	lis	r8, MI_RSV4I@h
>>>> -	ori	r8, r8, 0x1c00
>>>> -
>>>> +	lis	r8, MI_RESETVAL@h
>>>>    	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
>>>>    
>>>> -#ifdef CONFIG_PIN_TLB
>>>> -	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
>>>> -	ori	r10, r10, 0x1c00
>>>> -	mr	r8, r10
>>>> -#else
>>>>    	lis	r10, MD_RESETVAL@h
>>>> -#endif
>>>>    #ifndef CONFIG_8xx_COPYBACK
>>>>    	oris	r10, r10, MD_WTDEF@h
>>>>    #endif
>>>>    	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
>>>>    
>>>> +	tlbia			/* Invalidate all TLB entries */
>>> Is this change to make sure we invalidate everything even if the
>>> bootloader set RSV4I?
>> Most probably. It is step 2 of the process defined in MPC866 and MPC885
>> Reference Manuals:
>>
>> §8.10.3 Loading Locked TLB Entries:
>> The process of loading a single reserved entry in the TLB is as follows:
> To minimize code churn we should just fix actual problems, rather than
> shuffle things around to conform to a suggested sequence.  After all,
> we're not just trying to load a single entry.
Ok, I'll try again.
>
>>>> +	ori	r8, r8, 0x1c00
>>>> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
>>>> +#ifdef CONFIG_PIN_TLB
>>>> +	ori	r10, r10, 0x1c00
>>>> +	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
>>>> +#endif
>>> Still 0x1c00?
>> Yes, I kept the same entries in order to limit modifications:
>> * 28 = First 8Mbytes page
>> * 29 = IMMR
>> * 30 = Second 8Mbytes page
>> * 31 = Third 8Mbytes page
> If you actually want to program them in increasing order then it looks
> like you're still missing a write to CTR between the last two 8M entries
> -- thus you'll overwrite the IMMR with the last 8M entry.  That was the
> same problem that v1 fixed -- did that change get lost accidentally?
Oops, no, in fact I diffed from the version which was including it 
already. My mistake.
>
> The hardware wants to decrement; why fight it?
I see your point.
However it is not clear in the documentation if the decrement is done 
really after the update, or at xTLB interrupt. So I propose to still set 
the CTR ourself as described in the reference Manual and not assume that 
the HW decrements it.
>
>>>>    	/* Now map the lower 8 Meg into the TLBs.  For this quick hack,
>>>>    	 * we can load the instruction and data TLB registers with the
>>>>    	 * same values.
>>>> @@ -825,6 +822,12 @@
>>>>    	mtspr	SPRN_MI_AP, r8
>>>>    	mtspr	SPRN_MD_AP, r8
>>>>    
>>>> +	/* Always pin the first 8 MB ITLB to prevent ITLB
>>>> +	 * misses while mucking around with SRR0/SRR1 in asm
>>>> +	 */
>>>> +	lis	r8, (MI_RSV4I | MI_RESETVAL)@h
>>>> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
>>> Entry 0 is not pinnable.
>> Here we are not trying to pin entry 0.
> Sorry, misread the patch.
>
>> We are at step 8, we are setting
>> MI_RSV4I. At the same time, we set MD_CTR to 0 which is off the pinned
>> range, to be sure that we won't overwrite one of the pinned entries.
>>
>> The main difference compared to the previous implementation is that
>> before, we were setting the RSV4I bit before loading the TLB entries.
>> Now, as defined in the Reference Manuals, we are doing it at the end.
> Have you seen any evidence that it matters?
Not really.

Ok, propose a new patch in a few minutes.

Christophe
Scott Wood - Sept. 20, 2013, 9:22 p.m.
On Tue, 2013-09-17 at 18:40 +0200, leroy christophe wrote:
> Le 16/09/2013 23:02, Scott Wood a écrit :
> > On Fri, 2013-09-13 at 07:04 +0200, leroy christophe wrote:
> >> Le 12/09/2013 20:44, Scott Wood a écrit :
> >>> On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote:
> >>>> This is a reorganisation of the setup of the TLB at kernel startup, in order
> >>>> to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866
> >>>> and MPC885 reference manuals.
> >>>>
> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >>>>
> >>>> diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S
> >>>> --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S	2013-09-02 22:46:10.000000000 +0200
> >>>> +++ linux-3.11/arch/powerpc/kernel/head_8xx.S	2013-09-09 11:28:54.000000000 +0200
> >>>> @@ -785,27 +785,24 @@
> >>>>     * these mappings is mapped by page tables.
> >>>>     */
> >>>>    initial_mmu:
> >>>> -	tlbia			/* Invalidate all TLB entries */
> >>>> -/* Always pin the first 8 MB ITLB to prevent ITLB
> >>>> -   misses while mucking around with SRR0/SRR1 in asm
> >>>> -*/
> >>>> -	lis	r8, MI_RSV4I@h
> >>>> -	ori	r8, r8, 0x1c00
> >>>> -
> >>>> +	lis	r8, MI_RESETVAL@h
> >>>>    	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> >>>>    
> >>>> -#ifdef CONFIG_PIN_TLB
> >>>> -	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
> >>>> -	ori	r10, r10, 0x1c00
> >>>> -	mr	r8, r10
> >>>> -#else
> >>>>    	lis	r10, MD_RESETVAL@h
> >>>> -#endif
> >>>>    #ifndef CONFIG_8xx_COPYBACK
> >>>>    	oris	r10, r10, MD_WTDEF@h
> >>>>    #endif
> >>>>    	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
> >>>>    
> >>>> +	tlbia			/* Invalidate all TLB entries */
> >>> Is this change to make sure we invalidate everything even if the
> >>> bootloader set RSV4I?
> >> Most probably. It is step 2 of the process defined in MPC866 and MPC885
> >> Reference Manuals:
> >>
> >> §8.10.3 Loading Locked TLB Entries:
> >> The process of loading a single reserved entry in the TLB is as follows:
> > To minimize code churn we should just fix actual problems, rather than
> > shuffle things around to conform to a suggested sequence.  After all,
> > we're not just trying to load a single entry.
> Ok, I'll try again.
> >
> >>>> +	ori	r8, r8, 0x1c00
> >>>> +	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
> >>>> +#ifdef CONFIG_PIN_TLB
> >>>> +	ori	r10, r10, 0x1c00
> >>>> +	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
> >>>> +#endif
> >>> Still 0x1c00?
> >> Yes, I kept the same entries in order to limit modifications:
> >> * 28 = First 8Mbytes page
> >> * 29 = IMMR
> >> * 30 = Second 8Mbytes page
> >> * 31 = Third 8Mbytes page
> > If you actually want to program them in increasing order then it looks
> > like you're still missing a write to CTR between the last two 8M entries
> > -- thus you'll overwrite the IMMR with the last 8M entry.  That was the
> > same problem that v1 fixed -- did that change get lost accidentally?
> Oops, no, in fact I diffed from the version which was including it 
> already. My mistake.
> >
> > The hardware wants to decrement; why fight it?
> I see your point.
> However it is not clear in the documentation if the decrement is done 
> really after the update, or at xTLB interrupt. So I propose to still set 
> the CTR ourself as described in the reference Manual and not assume that 
> the HW decrements it.

It says "every update" -- do you have any reason to believe that's
wrong?  It could be tested...

-Scott
LEROY Christophe - Sept. 24, 2013, 8 a.m.
Le 20/09/2013 23:22, Scott Wood a écrit :
>>> The hardware wants to decrement; why fight it?
>> >I see your point.
>> >However it is not clear in the documentation if the decrement is done
>> >really after the update, or at xTLB interrupt. So I propose to still set
>> >the CTR ourself as described in the reference Manual and not assume that
>> >the HW decrements it.
> It says "every update" -- do you have any reason to believe that's
> wrong?  It could be tested...
>
>
Ok. I just test it,  and I observe the following: As we have set the 
RSV4x bit, the CPU sets Mx_CTR to a value below 0x1c after each update:
* After writing entry 0x1c, Mx_CTR has value 0x1b
* After writing entry 0x1d, Mx_CTR has value 0x18
* After writing entry 0x1e, Mx_CTR has value 0x19
* After writing entry 0x1f, Mx_CTR has value 0x1a

Indeed the first version of my patch was complete, only the description 
was not fully correct.
So, in order to minimise code churn, I will re-submit my initial patch 
with a modified description.

Christophe

Patch

diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S
--- linux-3.11.org/arch/powerpc/kernel/head_8xx.S	2013-09-02 22:46:10.000000000 +0200
+++ linux-3.11/arch/powerpc/kernel/head_8xx.S	2013-09-09 11:28:54.000000000 +0200
@@ -785,27 +785,24 @@ 
  * these mappings is mapped by page tables.
  */
 initial_mmu:
-	tlbia			/* Invalidate all TLB entries */
-/* Always pin the first 8 MB ITLB to prevent ITLB
-   misses while mucking around with SRR0/SRR1 in asm
-*/
-	lis	r8, MI_RSV4I@h
-	ori	r8, r8, 0x1c00
-
+	lis	r8, MI_RESETVAL@h
 	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
 
-#ifdef CONFIG_PIN_TLB
-	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
-	ori	r10, r10, 0x1c00
-	mr	r8, r10
-#else
 	lis	r10, MD_RESETVAL@h
-#endif
 #ifndef CONFIG_8xx_COPYBACK
 	oris	r10, r10, MD_WTDEF@h
 #endif
 	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
 
+	tlbia			/* Invalidate all TLB entries */
+
+	ori	r8, r8, 0x1c00
+	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
+#ifdef CONFIG_PIN_TLB
+	ori	r10, r10, 0x1c00
+	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
+#endif
+
 	/* Now map the lower 8 Meg into the TLBs.  For this quick hack,
 	 * we can load the instruction and data TLB registers with the
 	 * same values.
@@ -825,6 +822,12 @@ 
 	mtspr	SPRN_MI_AP, r8
 	mtspr	SPRN_MD_AP, r8
 
+	/* Always pin the first 8 MB ITLB to prevent ITLB
+	 * misses while mucking around with SRR0/SRR1 in asm
+	 */
+	lis	r8, (MI_RSV4I | MI_RESETVAL)@h
+	mtspr	SPRN_MI_CTR, r8	/* Set instruction MMU control */
+
 	/* Map another 8 MByte at the IMMR to get the processor
 	 * internal registers (among other things).
 	 */
@@ -870,7 +873,15 @@ 
 	mtspr	SPRN_MD_TWC, r9
 	addis	r11, r11, 0x0080	/* Add 8M */
 	mtspr	SPRN_MD_RPN, r11
+
+	lis	r10, (MD_RSV4I | MD_RESETVAL)@h
+#else
+	lis	r10, MD_RESETVAL@h
 #endif
+#ifndef CONFIG_8xx_COPYBACK
+	oris	r10, r10, MD_WTDEF@h
+#endif
+	mtspr	SPRN_MD_CTR, r10	/* Set data TLB control */
 
 	/* Since the cache is enabled according to the information we
 	 * just loaded into the TLB, invalidate and enable the caches here.