Patchwork powerpc 8xx: Fixing issue with CONFIG_PIN_TLB

login
register
mail settings
Submitter LEROY Christophe
Date Sept. 11, 2013, 4:44 p.m.
Message ID <201309111644.r8BGiuDZ016325@localhost.localdomain>
Download mbox | patch
Permalink /patch/274343/
State Accepted
Commit 4e591f3c0a618b8230a37bfff64b59e76374f2e5
Headers show

Comments

LEROY Christophe - Sept. 11, 2013, 4:44 p.m.
Activating CONFIG_PIN_TLB is supposed to pin the IMMR and the first three
8Mbytes pages. But the setting of the MD_CTR was missing so as the index is
decremented every DTLB update, the pinning of the third 8Mbytes page was
overwriting the DTLB entry for IMMR.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Scott Wood - Sept. 11, 2013, 10:36 p.m.
On Wed, 2013-09-11 at 18:44 +0200, Christophe Leroy wrote:
> Activating CONFIG_PIN_TLB is supposed to pin the IMMR and the first three
> 8Mbytes pages. But the setting of the MD_CTR was missing so as the index is
> decremented every DTLB update, the pinning of the third 8Mbytes page was
> overwriting the DTLB entry for IMMR.
> 
> 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
> @@ -862,6 +862,9 @@
>  	addis	r11, r11, 0x0080	/* Add 8M */
>  	mtspr	SPRN_MD_RPN, r11
>  
> +	addi	r10, r10, 0x0100
> +	mtspr	SPRN_MD_CTR, r10
> +
>  	addis	r8, r8, 0x0080		/* Add 8M */
>  	mtspr	SPRN_MD_EPN, r8
>  	mtspr	SPRN_MD_TWC, r9

I wonder why we don't start from entry 31 so we can actually make use of
that autodecrement.  What will happen when we load the first normal TLB
entry later on?  I don't see any setting of SPRN_MD_CTR after this code,
so won't it overwrite entry 30 (the middle 8M) in the CONFIG_PIN_TLB
case?

Ben, would patches like this be considered bugfixes as far as merging
goes, or would they be for next given that it's something that's never
really worked right and hasn't been touched in years?

-Scott
Benjamin Herrenschmidt - Sept. 12, 2013, 12:15 a.m.
On Wed, 2013-09-11 at 17:36 -0500, Scott Wood wrote:
> I wonder why we don't start from entry 31 so we can actually make use of
> that autodecrement.  What will happen when we load the first normal TLB
> entry later on?  I don't see any setting of SPRN_MD_CTR after this code,
> so won't it overwrite entry 30 (the middle 8M) in the CONFIG_PIN_TLB
> case?
> 
> Ben, would patches like this be considered bugfixes as far as merging
> goes, or would they be for next given that it's something that's never
> really worked right and hasn't been touched in years?

Since they don't affect anything outside of 8xx, I'm happy to take them
until around -rc2 or 3. But it's your call really.

Cheers,
Ben.
LEROY Christophe - Sept. 12, 2013, 5:57 a.m.
Le 12/09/2013 02:15, Benjamin Herrenschmidt a écrit :
> On Wed, 2013-09-11 at 17:36 -0500, Scott Wood wrote:
>> I wonder why we don't start from entry 31 so we can actually make use of
>> that autodecrement.  What will happen when we load the first normal TLB
>> entry later on?  I don't see any setting of SPRN_MD_CTR after this code,
>> so won't it overwrite entry 30 (the middle 8M) in the CONFIG_PIN_TLB
>> case?
>>
>> Ben, would patches like this be considered bugfixes as far as merging
>> goes, or would they be for next given that it's something that's never
>> really worked right and hasn't been touched in years?
> Since they don't affect anything outside of 8xx, I'm happy to take them
> until around -rc2 or 3. But it's your call really.
>
>
Scott, you're right, I didn't see that other consequence.
I'll come with a more complete patch this afternoon.
Thanks

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
@@ -862,6 +862,9 @@ 
 	addis	r11, r11, 0x0080	/* Add 8M */
 	mtspr	SPRN_MD_RPN, r11
 
+	addi	r10, r10, 0x0100
+	mtspr	SPRN_MD_CTR, r10
+
 	addis	r8, r8, 0x0080		/* Add 8M */
 	mtspr	SPRN_MD_EPN, r8
 	mtspr	SPRN_MD_TWC, r9