[PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[Christophe Leroy]

Today, the only way to load kernels whose size is greater than 8Mbytes is to
activate CONFIG_PIN_TLB. Otherwise, the physical memory initially mapped is
limited to 8Mbytes. This patch adds the capability to select the size of initial
memory between 8/16/24 Mbytes and this is regardless of whether CONFIG_PIN_TLB
is active or not. It allows to load "big" kernels (for instance when activating
CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -980,6 +980,29 @@
 config PIN_TLB
 	bool "Pinned Kernel TLBs (860 ONLY)"
 	depends on ADVANCED_OPTIONS && 8xx
+
+choice
+	prompt "Initial Data Memory Mapped on 8xx"
+	default 8xx_MAP_8M
+	depends on ADVANCED_OPTIONS && 8xx
+
+config	8xx_INIT_MAP_8M
+	bool "8 Mbytes"
+
+config	8xx_INIT_MAP_16M
+	bool "16 Mbytes"
+
+config	8xx_INIT_MAP_24M
+	bool "24 Mbytes"
+
+endchoice
+
+config 8xx_INIT_MAP
+	hex
+	default 0x01800000 if 8xx_INIT_MAP_24M
+	default 0x01000000 if 8xx_INIT_MAP_16M
+	default 0x00800000
+
 endmenu
 
 if PPC64
diff -ur a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -841,11 +841,13 @@
 	ori	r8, r8, MI_BOOTINIT|0x2 /* Inhibit cache -- Cort */
 	mtspr	SPRN_MD_RPN, r8
 
-#ifdef CONFIG_PIN_TLB
+#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_MAP_24M)
 	/* Map two more 8M kernel data pages.
 	*/
+#ifdef CONFIG_PIN_TLB
 	addi	r10, r10, 0x0100
 	mtspr	SPRN_MD_CTR, r10
+#endif
 
 	lis	r8, KERNELBASE@h	/* Create vaddr for TLB */
 	addis	r8, r8, 0x0080		/* Add 8M */
@@ -858,15 +860,19 @@
 	addis	r11, r11, 0x0080	/* Add 8M */
 	mtspr	SPRN_MD_RPN, r11
 
+#ifdef CONFIG_8xx_INIT_MAP_24M
+#ifdef CONFIG_PIN_TLB
 	addi	r10, r10, 0x0100
 	mtspr	SPRN_MD_CTR, r10
+#endif
 
 	addis	r8, r8, 0x0080		/* Add 8M */
 	mtspr	SPRN_MD_EPN, r8
 	mtspr	SPRN_MD_TWC, r9
 	addis	r11, r11, 0x0080	/* Add 8M */
 	mtspr	SPRN_MD_RPN, r11
-#endif
+#endif /* CONFIG_8xx_INIT_MAP_24M */
+#endif /* CONFIG_8xx_INIT_MAP_16M || CONFIG_8xx_INIT_MAP_24M */
 
 	/* Since the cache is enabled according to the information we
 	 * just loaded into the TLB, invalidate and enable the caches here.
diff -ur a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -213,12 +213,8 @@
 	 */
 	BUG_ON(first_memblock_base != 0);
 
-#ifdef CONFIG_PIN_TLB
 	/* 8xx can only access 24MB at the moment */
-	memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01800000));
-#else
-	/* 8xx can only access 8MB at the moment */
-	memblock_set_current_limit(min_t(u64, first_memblock_size, 0x00800000));
-#endif
+	memblock_set_current_limit(min_t(u64, first_memblock_size,
+		CONFIG_8xx_INIT_MAP));
 }
 #endif /* CONFIG_8xx */

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[Scott Wood]

On Tue, 2013-12-10 at 12:29 +0100, Christophe Leroy wrote:
> Today, the only way to load kernels whose size is greater than 8Mbytes is to
> activate CONFIG_PIN_TLB. Otherwise, the physical memory initially mapped is
> limited to 8Mbytes. This patch adds the capability to select the size of initial
> memory between 8/16/24 Mbytes and this is regardless of whether CONFIG_PIN_TLB
> is active or not. It allows to load "big" kernels (for instance when activating
> CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 
> diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -980,6 +980,29 @@
>  config PIN_TLB
>  	bool "Pinned Kernel TLBs (860 ONLY)"
>  	depends on ADVANCED_OPTIONS && 8xx
> +
> +choice
> +	prompt "Initial Data Memory Mapped on 8xx"
> +	default 8xx_MAP_8M
> +	depends on ADVANCED_OPTIONS && 8xx
> +
> +config	8xx_INIT_MAP_8M
> +	bool "8 Mbytes"
> +
> +config	8xx_INIT_MAP_16M
> +	bool "16 Mbytes"
> +
> +config	8xx_INIT_MAP_24M
> +	bool "24 Mbytes"

Are you working with a loader that passes initial-mapped-area size in r7
as per ePAPR?  If so, we could rely on that at runtime.  If you're using
a non-ancient U-Boot, it should qualify here even if it's not fully
ePAPR compliant (it passes the value of the bootm_mapsize variable in
r7).

> -#ifdef CONFIG_PIN_TLB
> +#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_MAP_24M)
>  	/* Map two more 8M kernel data pages.
>  	*/
> +#ifdef CONFIG_PIN_TLB
>  	addi	r10, r10, 0x0100
>  	mtspr	SPRN_MD_CTR, r10
> +#endif
>  
>  	lis	r8, KERNELBASE@h	/* Create vaddr for TLB */
>  	addis	r8, r8, 0x0080		/* Add 8M */
> @@ -858,15 +860,19 @@
>  	addis	r11, r11, 0x0080	/* Add 8M */
>  	mtspr	SPRN_MD_RPN, r11
>  
> +#ifdef CONFIG_8xx_INIT_MAP_24M
> +#ifdef CONFIG_PIN_TLB
>  	addi	r10, r10, 0x0100
>  	mtspr	SPRN_MD_CTR, r10
> +#endif

Are these ifdefs for CONFIG_PIN_TLB really needed?  It shouldn't harm
anything to use those entries even if they're not being pinned.
 
-Scott

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[leroy christophe]

Le 10/12/2013 23:24, Scott Wood a écrit :
> On Tue, 2013-12-10 at 12:29 +0100, Christophe Leroy wrote:
>> Today, the only way to load kernels whose size is greater than 8Mbytes is to
>> activate CONFIG_PIN_TLB. Otherwise, the physical memory initially mapped is
>> limited to 8Mbytes. This patch adds the capability to select the size of initial
>> memory between 8/16/24 Mbytes and this is regardless of whether CONFIG_PIN_TLB
>> is active or not. It allows to load "big" kernels (for instance when activating
>> CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -980,6 +980,29 @@
>>   config PIN_TLB
>>   	bool "Pinned Kernel TLBs (860 ONLY)"
>>   	depends on ADVANCED_OPTIONS && 8xx
>> +
>> +choice
>> +	prompt "Initial Data Memory Mapped on 8xx"
>> +	default 8xx_MAP_8M
>> +	depends on ADVANCED_OPTIONS && 8xx
>> +
>> +config	8xx_INIT_MAP_8M
>> +	bool "8 Mbytes"
>> +
>> +config	8xx_INIT_MAP_16M
>> +	bool "16 Mbytes"
>> +
>> +config	8xx_INIT_MAP_24M
>> +	bool "24 Mbytes"
> Are you working with a loader that passes initial-mapped-area size in r7
> as per ePAPR?  If so, we could rely on that at runtime.  If you're using
> a non-ancient U-Boot, it should qualify here even if it's not fully
> ePAPR compliant (it passes the value of the bootm_mapsize variable in
> r7).
Ok, let me check that. But it means that the size of the kernel I can 
boot will depend on the initial memory mapped by uboot ? Isn't it 
limitating ?
Even if uboot only maps 8Mbytes, why couldn't I be allowed to boot a 
kernel having 10 Mbytes data if I have 32 Mbytes mem on the board ?
I don't like the idea of having to change the bootloader just because I 
want to activate CONFIG_LOCKDEP to debug my kernel.
>
>> -#ifdef CONFIG_PIN_TLB
>> +#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_MAP_24M)
>>   	/* Map two more 8M kernel data pages.
>>   	*/
>> +#ifdef CONFIG_PIN_TLB
>>   	addi	r10, r10, 0x0100
>>   	mtspr	SPRN_MD_CTR, r10
>> +#endif
>>   
>>   	lis	r8, KERNELBASE@h	/* Create vaddr for TLB */
>>   	addis	r8, r8, 0x0080		/* Add 8M */
>> @@ -858,15 +860,19 @@
>>   	addis	r11, r11, 0x0080	/* Add 8M */
>>   	mtspr	SPRN_MD_RPN, r11
>>   
>> +#ifdef CONFIG_8xx_INIT_MAP_24M
>> +#ifdef CONFIG_PIN_TLB
>>   	addi	r10, r10, 0x0100
>>   	mtspr	SPRN_MD_CTR, r10
>> +#endif
> Are these ifdefs for CONFIG_PIN_TLB really needed?  It shouldn't harm
> anything to use those entries even if they're not being pinned.

I'm not sure I understand your comment.
ifdef for CONFIG_PIN_TLB was already there before, but was enclosing the 
whole block, so 24 Mbytes were automatically mapped when you selected 
CONFIG_PIN_TLB and only 8 Mbytes were mapped when you didn't select 
CONFIG_PIN_TLB.
I reduced the scope of those ifdefs so that they now apply on the 
pinning only.

Christophe

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[Scott Wood]

On Wed, 2013-12-11 at 00:05 +0100, leroy christophe wrote:
> Le 10/12/2013 23:24, Scott Wood a =C3=A9crit :
> > On Tue, 2013-12-10 at 12:29 +0100, Christophe Leroy wrote:
> >> Today, the only way to load kernels whose size is greater than 8Mbyt=
es is to
> >> activate CONFIG_PIN_TLB. Otherwise, the physical memory initially ma=
pped is
> >> limited to 8Mbytes. This patch adds the capability to select the siz=
e of initial
> >> memory between 8/16/24 Mbytes and this is regardless of whether CONF=
IG_PIN_TLB
> >> is active or not. It allows to load "big" kernels (for instance when=
 activating
> >> CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >>
> >> diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> --- a/arch/powerpc/Kconfig
> >> +++ b/arch/powerpc/Kconfig
> >> @@ -980,6 +980,29 @@
> >>   config PIN_TLB
> >>   	bool "Pinned Kernel TLBs (860 ONLY)"
> >>   	depends on ADVANCED_OPTIONS && 8xx
> >> +
> >> +choice
> >> +	prompt "Initial Data Memory Mapped on 8xx"
> >> +	default 8xx_MAP_8M
> >> +	depends on ADVANCED_OPTIONS && 8xx
> >> +
> >> +config	8xx_INIT_MAP_8M
> >> +	bool "8 Mbytes"
> >> +
> >> +config	8xx_INIT_MAP_16M
> >> +	bool "16 Mbytes"
> >> +
> >> +config	8xx_INIT_MAP_24M
> >> +	bool "24 Mbytes"
> > Are you working with a loader that passes initial-mapped-area size in=
 r7
> > as per ePAPR?  If so, we could rely on that at runtime.  If you're us=
ing
> > a non-ancient U-Boot, it should qualify here even if it's not fully
> > ePAPR compliant (it passes the value of the bootm_mapsize variable in
> > r7).
> Ok, let me check that. But it means that the size of the kernel I can=20
> boot will depend on the initial memory mapped by uboot ? Isn't it=20
> limitating ?

The ePAPR IMA is supposed to be large enough to include the OS image,
device tree, etc.

> Even if uboot only maps 8Mbytes, why couldn't I be allowed to boot a=20
> kernel having 10 Mbytes data if I have 32 Mbytes mem on the board ?
> I don't like the idea of having to change the bootloader just because I=
=20
> want to activate CONFIG_LOCKDEP to debug my kernel.

Well, as noted, if you're using a non-ancient U-Boot you shouldn't have
to change anything because it already implements r7.  Now, the value of
r7 it passes might be a lie as far as ePAPR is concerned, since it's
supposed to represent what's actually mapped, but that's another matter.

Even fixing that wouldn't mean you have to change U-Boot every time the
kernel size changes; you'd just set it to something reasonable and be
done with it.  I'm not fond of adding kconfigs to hack around a problem
that has already been addressed in the standard that governs the PPC
boot process that U-Boot claims to implement.

> >> -#ifdef CONFIG_PIN_TLB
> >> +#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_M=
AP_24M)
> >>   	/* Map two more 8M kernel data pages.
> >>   	*/
> >> +#ifdef CONFIG_PIN_TLB
> >>   	addi	r10, r10, 0x0100
> >>   	mtspr	SPRN_MD_CTR, r10
> >> +#endif
> >>  =20
> >>   	lis	r8, KERNELBASE@h	/* Create vaddr for TLB */
> >>   	addis	r8, r8, 0x0080		/* Add 8M */
> >> @@ -858,15 +860,19 @@
> >>   	addis	r11, r11, 0x0080	/* Add 8M */
> >>   	mtspr	SPRN_MD_RPN, r11
> >>  =20
> >> +#ifdef CONFIG_8xx_INIT_MAP_24M
> >> +#ifdef CONFIG_PIN_TLB
> >>   	addi	r10, r10, 0x0100
> >>   	mtspr	SPRN_MD_CTR, r10
> >> +#endif
> > Are these ifdefs for CONFIG_PIN_TLB really needed?  It shouldn't harm
> > anything to use those entries even if they're not being pinned.
>=20
> I'm not sure I understand your comment.
> ifdef for CONFIG_PIN_TLB was already there before, but was enclosing th=
e=20
> whole block, so 24 Mbytes were automatically mapped when you selected=20
> CONFIG_PIN_TLB and only 8 Mbytes were mapped when you didn't select=20
> CONFIG_PIN_TLB.
> I reduced the scope of those ifdefs so that they now apply on the=20
> pinning only.

There wasn't previously an ifdef specifically around the setting of
SPRN_MD_CTR.  That's new.  There was an ifdef around the entire block,
which has gone away because you are now trying to map more than 8M
regardless of CONFIG_PIN_TLB, but that has nothing to do with whether
there should be an ifdef around SPRN_MD_CTR.

-Scott

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[leroy christophe]

Le 11/12/2013 00:18, Scott Wood a écrit :
> On Wed, 2013-12-11 at 00:05 +0100, leroy christophe wrote:
>> Le 10/12/2013 23:24, Scott Wood a écrit :
>>> On Tue, 2013-12-10 at 12:29 +0100, Christophe Leroy wrote:
>>>> Today, the only way to load kernels whose size is greater than 8Mbytes is to
>>>> activate CONFIG_PIN_TLB. Otherwise, the physical memory initially mapped is
>>>> limited to 8Mbytes. This patch adds the capability to select the size of initial
>>>> memory between 8/16/24 Mbytes and this is regardless of whether CONFIG_PIN_TLB
>>>> is active or not. It allows to load "big" kernels (for instance when activating
>>>> CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>
>>>> diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>> --- a/arch/powerpc/Kconfig
>>>> +++ b/arch/powerpc/Kconfig
>>>> @@ -980,6 +980,29 @@
>>>>    config PIN_TLB
>>>>    	bool "Pinned Kernel TLBs (860 ONLY)"
>>>>    	depends on ADVANCED_OPTIONS && 8xx
>>>> +
>>>> +choice
>>>> +	prompt "Initial Data Memory Mapped on 8xx"
>>>> +	default 8xx_MAP_8M
>>>> +	depends on ADVANCED_OPTIONS && 8xx
>>>> +
>>>> +config	8xx_INIT_MAP_8M
>>>> +	bool "8 Mbytes"
>>>> +
>>>> +config	8xx_INIT_MAP_16M
>>>> +	bool "16 Mbytes"
>>>> +
>>>> +config	8xx_INIT_MAP_24M
>>>> +	bool "24 Mbytes"
>>> Are you working with a loader that passes initial-mapped-area size in r7
>>> as per ePAPR?  If so, we could rely on that at runtime.  If you're using
>>> a non-ancient U-Boot, it should qualify here even if it's not fully
>>> ePAPR compliant (it passes the value of the bootm_mapsize variable in
>>> r7).
>> Ok, let me check that. But it means that the size of the kernel I can
>> boot will depend on the initial memory mapped by uboot ? Isn't it
>> limitating ?
> The ePAPR IMA is supposed to be large enough to include the OS image,
> device tree, etc.
>
>> Even if uboot only maps 8Mbytes, why couldn't I be allowed to boot a
>> kernel having 10 Mbytes data if I have 32 Mbytes mem on the board ?
>> I don't like the idea of having to change the bootloader just because I
>> want to activate CONFIG_LOCKDEP to debug my kernel.
> Well, as noted, if you're using a non-ancient U-Boot you shouldn't have
> to change anything because it already implements r7.  Now, the value of
> r7 it passes might be a lie as far as ePAPR is concerned, since it's
> supposed to represent what's actually mapped, but that's another matter.
>
> Even fixing that wouldn't mean you have to change U-Boot every time the
> kernel size changes; you'd just set it to something reasonable and be
> done with it.  I'm not fond of adding kconfigs to hack around a problem
> that has already been addressed in the standard that governs the PPC
> boot process that U-Boot claims to implement.
Well, ok, that makes sense. I'll investigate around that solution.
>
>>>> -#ifdef CONFIG_PIN_TLB
>>>> +#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_MAP_24M)
>>>>    	/* Map two more 8M kernel data pages.
>>>>    	*/
>>>> +#ifdef CONFIG_PIN_TLB
>>>>    	addi	r10, r10, 0x0100
>>>>    	mtspr	SPRN_MD_CTR, r10
>>>> +#endif
>>>>    
>>>>    	lis	r8, KERNELBASE@h	/* Create vaddr for TLB */
>>>>    	addis	r8, r8, 0x0080		/* Add 8M */
>>>> @@ -858,15 +860,19 @@
>>>>    	addis	r11, r11, 0x0080	/* Add 8M */
>>>>    	mtspr	SPRN_MD_RPN, r11
>>>>    
>>>> +#ifdef CONFIG_8xx_INIT_MAP_24M
>>>> +#ifdef CONFIG_PIN_TLB
>>>>    	addi	r10, r10, 0x0100
>>>>    	mtspr	SPRN_MD_CTR, r10
>>>> +#endif
>>> Are these ifdefs for CONFIG_PIN_TLB really needed?  It shouldn't harm
>>> anything to use those entries even if they're not being pinned.
>> I'm not sure I understand your comment.
>> ifdef for CONFIG_PIN_TLB was already there before, but was enclosing the
>> whole block, so 24 Mbytes were automatically mapped when you selected
>> CONFIG_PIN_TLB and only 8 Mbytes were mapped when you didn't select
>> CONFIG_PIN_TLB.
>> I reduced the scope of those ifdefs so that they now apply on the
>> pinning only.
> There wasn't previously an ifdef specifically around the setting of
> SPRN_MD_CTR.  That's new.  There was an ifdef around the entire block,
> which has gone away because you are now trying to map more than 8M
> regardless of CONFIG_PIN_TLB, but that has nothing to do with whether
> there should be an ifdef around SPRN_MD_CTR.
>
>
Euh, ok, but then we have to fix it in the whole function, not only in 
this block. Do you think it is worth doing it ?
Then we are back to the problem we discussed some months ago which is 
that the 8xx is decrementing the MD_CTR after writting a TLB entry, and 
if pinning is activated it decrements it out of the pinnable area. So it 
would still be needed to:
* Reposition it for each entry for when the pinning is activated
* Make sure we set it out of the area at the end when the pinning is not 
active hence the area not protected.
* Then we should probably reverse the entries, start at 31 and go down 
to 28 instead of going from 28 to 31 as do today.
But is it worth doing such a big change which will not add anything 
functionnaly speaking ?

Christophe

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[Scott Wood]

On Wed, 2013-12-11 at 00:36 +0100, leroy christophe wrote:
> Le 11/12/2013 00:18, Scott Wood a écrit :
> > There wasn't previously an ifdef specifically around the setting of
> > SPRN_MD_CTR.  That's new.  There was an ifdef around the entire block,
> > which has gone away because you are now trying to map more than 8M
> > regardless of CONFIG_PIN_TLB, but that has nothing to do with whether
> > there should be an ifdef around SPRN_MD_CTR.
> >
> >
> Euh, ok, but then we have to fix it in the whole function, not only in 
> this block. Do you think it is worth doing it ?

Fix what in the whole function?  I was asking what harm there would be
if you just remove all the CONFIG_PIN_TLB ifdefs except around the
actual RSV4I setting -- do we really care what value goes in MD_CTR for
the non-pinned case, as long as it's different for each entry?

-Scott

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[leroy christophe]

Le 16/12/2013 23:57, Scott Wood a écrit :
> On Wed, 2013-12-11 at 00:36 +0100, leroy christophe wrote:
>> Le 11/12/2013 00:18, Scott Wood a écrit :
>>> There wasn't previously an ifdef specifically around the setting of
>>> SPRN_MD_CTR.  That's new.  There was an ifdef around the entire block,
>>> which has gone away because you are now trying to map more than 8M
>>> regardless of CONFIG_PIN_TLB, but that has nothing to do with whether
>>> there should be an ifdef around SPRN_MD_CTR.
>>>
>>>
>> Euh, ok, but then we have to fix it in the whole function, not only in
>> this block. Do you think it is worth doing it ?
> Fix what in the whole function?  I was asking what harm there would be
> if you just remove all the CONFIG_PIN_TLB ifdefs except around the
> actual RSV4I setting -- do we really care what value goes in MD_CTR for
> the non-pinned case, as long as it's different for each entry?
>
>
MD_CTR is decremented after each entry added.
However, the function populates entry 28, then 29, then 30, then 31. At 
the end MD_CTR has then value 30, ready to overide entry 30 then 29 then 
28 then 27 .....

So I will remove all the CONFIG_PIN_TLB, but I'll also have to fix the 
value set in MD_CTR to start from 31, won't I ?

Do you have any comment/recommendation on my tentative v3 patch where I 
have tried to implement based on the use of r7 as you recommended ?

Christophe

Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB

[Scott Wood]

On Tue, 2013-12-17 at 06:54 +0100, leroy christophe wrote:
> Le 16/12/2013 23:57, Scott Wood a écrit :
> > On Wed, 2013-12-11 at 00:36 +0100, leroy christophe wrote:
> >> Le 11/12/2013 00:18, Scott Wood a écrit :
> >>> There wasn't previously an ifdef specifically around the setting of
> >>> SPRN_MD_CTR.  That's new.  There was an ifdef around the entire block,
> >>> which has gone away because you are now trying to map more than 8M
> >>> regardless of CONFIG_PIN_TLB, but that has nothing to do with whether
> >>> there should be an ifdef around SPRN_MD_CTR.
> >>>
> >>>
> >> Euh, ok, but then we have to fix it in the whole function, not only in
> >> this block. Do you think it is worth doing it ?
> > Fix what in the whole function?  I was asking what harm there would be
> > if you just remove all the CONFIG_PIN_TLB ifdefs except around the
> > actual RSV4I setting -- do we really care what value goes in MD_CTR for
> > the non-pinned case, as long as it's different for each entry?
> >
> >
> MD_CTR is decremented after each entry added.
> However, the function populates entry 28, then 29, then 30, then 31. At 
> the end MD_CTR has then value 30, ready to overide entry 30 then 29 then 
> 28 then 27 .....
> 
> So I will remove all the CONFIG_PIN_TLB, but I'll also have to fix the 
> value set in MD_CTR to start from 31, won't I ?

OK, so the answer is that we rely on autodecrement avoiding entries over
28 in the CONFIG_PIN_TLB case.  Leave the ifdefs, then.

> Do you have any comment/recommendation on my tentative v3 patch where I 
> have tried to implement based on the use of r7 as you recommended ?

I haven't reviewed it yet.

-Scott