[PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[Christophe Leroy]

Having a macro will help keep clear code.

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

---
v2: no change

 arch/powerpc/kernel/head_8xx.S | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index a1571b3..065896f 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -297,6 +297,17 @@ SystemCall:
  * We have to use the MD_xxx registers for the tablewalk because the
  * equivalent MI_xxx registers only perform the attribute functions.
  */
+
+#ifdef CONFIG_8xx_CPU15
+#define DO_8xx_CPU15(tmp, addr)	\
+	addi	tmp, addr, PAGE_SIZE;	\
+	tlbie	tmp;			\
+	addi	tmp, addr, PAGE_SIZE;	\
+	tlbie	tmp
+#else
+#define DO_8xx_CPU15(tmp, addr)
+#endif
+
 InstructionTLBMiss:
 #ifdef CONFIG_8xx_CPU6
 	mtspr	SPRN_DAR, r3
@@ -304,12 +315,7 @@ InstructionTLBMiss:
 	EXCEPTION_PROLOG_0
 	mtspr	SPRN_SPRG_SCRATCH2, r10
 	mfspr	r10, SPRN_SRR0	/* Get effective address of fault */
-#ifdef CONFIG_8xx_CPU15
-	addi	r11, r10, PAGE_SIZE
-	tlbie	r11
-	addi	r11, r10, -PAGE_SIZE
-	tlbie	r11
-#endif
+	DO_8xx_CPU15(r11, r10)
 
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
-- 
2.1.0

[PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[Christophe Leroy]

Having a macro will help keep clear code.

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

---
v2: no change

 arch/powerpc/kernel/head_8xx.S | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index a1571b3..065896f 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -297,6 +297,17 @@ SystemCall:
  * We have to use the MD_xxx registers for the tablewalk because the
  * equivalent MI_xxx registers only perform the attribute functions.
  */
+
+#ifdef CONFIG_8xx_CPU15
+#define DO_8xx_CPU15(tmp, addr)	\
+	addi	tmp, addr, PAGE_SIZE;	\
+	tlbie	tmp;			\
+	addi	tmp, addr, PAGE_SIZE;	\
+	tlbie	tmp
+#else
+#define DO_8xx_CPU15(tmp, addr)
+#endif
+
 InstructionTLBMiss:
 #ifdef CONFIG_8xx_CPU6
 	mtspr	SPRN_DAR, r3
@@ -304,12 +315,7 @@ InstructionTLBMiss:
 	EXCEPTION_PROLOG_0
 	mtspr	SPRN_SPRG_SCRATCH2, r10
 	mfspr	r10, SPRN_SRR0	/* Get effective address of fault */
-#ifdef CONFIG_8xx_CPU15
-	addi	r11, r10, PAGE_SIZE
-	tlbie	r11
-	addi	r11, r10, -PAGE_SIZE
-	tlbie	r11
-#endif
+	DO_8xx_CPU15(r11, r10)
 
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
-- 
2.1.0

RE: [PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[David Laight]

RnJvbSBDaHJpc3RvcGhlIExlcm95DQo+IEhhdmluZyBhIG1hY3JvIHdpbGwgaGVscCBrZWVwIGNs
ZWFyIGNvZGUuDQoNCkl0IG1pZ2h0IHJlbW92ZSBhbiAjaWYgYnV0IGl0IGRvZXNuJ3QgcmVhbGx5
IGhlbHAuDQpBbGwgaXQgbWVhbnMgaXMgdGhhdCBhbnlvbmUgcmVhZGluZyB0aGUgY29kZSBoYXMg
dG8gaHVudCBmb3INCnRoZSBkZWZpbml0aW9uIGJlZm9yZSBwcm9jZWVkaW5nLg0KDQpTb21lIGNv
bW1lbnQgYWJvdXQgd2hhdCAoYW5kIHdoeSkgdGhlIGV4dHJhIGNvZGUgaXMgbmVlZGVkDQptaWdo
dCBoZWxwLg0KDQouLi4NCj4gKw0KPiArI2lmZGVmIENPTkZJR184eHhfQ1BVMTUNCj4gKyNkZWZp
bmUgRE9fOHh4X0NQVTE1KHRtcCwgYWRkcikJXA0KPiArCWFkZGkJdG1wLCBhZGRyLCBQQUdFX1NJ
WkU7CVwNCj4gKwl0bGJpZQl0bXA7CQkJXA0KPiArCWFkZGkJdG1wLCBhZGRyLCBQQUdFX1NJWkU7
CVwNCg0KWW91J3ZlIGV2ZW4gdHJhbnNjcmliZWQgdGhpcyBpbmNvcnJlY3RseS4NCg0KQ2xlYXJs
eSBub3QgdGVzdGVkIDotKQ0KDQoJRGF2aWQNCg0KPiArCXRsYmllCXRtcA0KPiArI2Vsc2UNCj4g
KyNkZWZpbmUgRE9fOHh4X0NQVTE1KHRtcCwgYWRkcikNCj4gKyNlbmRpZg0KPiArDQo+ICBJbnN0
cnVjdGlvblRMQk1pc3M6DQo+ICAjaWZkZWYgQ09ORklHXzh4eF9DUFU2DQo+ICAJbXRzcHIJU1BS
Tl9EQVIsIHIzDQo+IEBAIC0zMDQsMTIgKzMxNSw3IEBAIEluc3RydWN0aW9uVExCTWlzczoNCj4g
IAlFWENFUFRJT05fUFJPTE9HXzANCj4gIAltdHNwcglTUFJOX1NQUkdfU0NSQVRDSDIsIHIxMA0K
PiAgCW1mc3ByCXIxMCwgU1BSTl9TUlIwCS8qIEdldCBlZmZlY3RpdmUgYWRkcmVzcyBvZiBmYXVs
dCAqLw0KPiAtI2lmZGVmIENPTkZJR184eHhfQ1BVMTUNCj4gLQlhZGRpCXIxMSwgcjEwLCBQQUdF
X1NJWkUNCj4gLQl0bGJpZQlyMTENCj4gLQlhZGRpCXIxMSwgcjEwLCAtUEFHRV9TSVpFDQo+IC0J
dGxiaWUJcjExDQo+IC0jZW5kaWYNCj4gKwlET184eHhfQ1BVMTUocjExLCByMTApDQo+IA0KPiAg
CS8qIElmIHdlIGFyZSBmYXVsdGluZyBhIGtlcm5lbCBhZGRyZXNzLCB3ZSBoYXZlIHRvIHVzZSB0
aGUNCj4gIAkgKiBrZXJuZWwgcGFnZSB0YWJsZXMuDQo+IC0tDQo+IDIuMS4wDQo+IA0KPiBfX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXw0KPiBMaW51eHBwYy1k
ZXYgbWFpbGluZyBsaXN0DQo+IExpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnDQo+IGh0dHBz
Oi8vbGlzdHMub3psYWJzLm9yZy9saXN0aW5mby9saW51eHBwYy1kZXYNCg==

RE: [PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[David Laight]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1405 bytes --]

>From Christophe Leroy
> Having a macro will help keep clear code.

It might remove an #if but it doesn't really help.
All it means is that anyone reading the code has to hunt for
the definition before proceeding.

Some comment about what (and why) the extra code is needed
might help.

...
> +
> +#ifdef CONFIG_8xx_CPU15
> +#define DO_8xx_CPU15(tmp, addr)	\
> +	addi	tmp, addr, PAGE_SIZE;	\
> +	tlbie	tmp;			\
> +	addi	tmp, addr, PAGE_SIZE;	\

You've even transcribed this incorrectly.

Clearly not tested :-)

	David

> +	tlbie	tmp
> +#else
> +#define DO_8xx_CPU15(tmp, addr)
> +#endif
> +
>  InstructionTLBMiss:
>  #ifdef CONFIG_8xx_CPU6
>  	mtspr	SPRN_DAR, r3
> @@ -304,12 +315,7 @@ InstructionTLBMiss:
>  	EXCEPTION_PROLOG_0
>  	mtspr	SPRN_SPRG_SCRATCH2, r10
>  	mfspr	r10, SPRN_SRR0	/* Get effective address of fault */
> -#ifdef CONFIG_8xx_CPU15
> -	addi	r11, r10, PAGE_SIZE
> -	tlbie	r11
> -	addi	r11, r10, -PAGE_SIZE
> -	tlbie	r11
> -#endif
> +	DO_8xx_CPU15(r11, r10)
> 
>  	/* If we are faulting a kernel address, we have to use the
>  	 * kernel page tables.
> --
> 2.1.0
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[leroy christophe]

Le 20/01/2015 12:09, David Laight a écrit :
>  From Christophe Leroy
>> Having a macro will help keep clear code.
> It might remove an #if but it doesn't really help.
> All it means is that anyone reading the code has to hunt for
> the definition before proceeding.
>
> Some comment about what (and why) the extra code is needed
> might help.
The main reason is because of patch 09/11 where we have to duplicate 
this code. I prefer to just duplicate one line rather than duplicate the 
whole code (especially because in v1 of the PATCHset, it was duplicated 
twice):

-    DO_8xx_CPU15(r11, r10)
[...]
#ifdef CONFIG_MODULES
[...]
+    DO_8xx_CPU15(r10, r11)
[...]
+#else
+    mfspr    r10, SPRN_SRR0    /* Get effective address of fault */
+    DO_8xx_CPU15(r11, r10)

Is this approach wrong ?


>
> ...
>> +
>> +#ifdef CONFIG_8xx_CPU15
>> +#define DO_8xx_CPU15(tmp, addr)	\
>> +	addi	tmp, addr, PAGE_SIZE;	\
>> +	tlbie	tmp;			\
>> +	addi	tmp, addr, PAGE_SIZE;	\
> You've even transcribed this incorrectly.
Oops
>
> Clearly not tested :-)
Indeed it's been tested, but tests can only show that the code is not 
worth than before.
This code is there to fix a chip errata which (almost?) never happens.
In my production version, I have not activated this errata, and he have 
never seen the problem on any of the more than 200 boards that have run 
for at least 4 years.

Christophe
>
> 	David
>
>> +	tlbie	tmp
>> +#else
>> +#define DO_8xx_CPU15(tmp, addr)
>> +#endif
>> +
>>   InstructionTLBMiss:
>>   #ifdef CONFIG_8xx_CPU6
>>   	mtspr	SPRN_DAR, r3
>> @@ -304,12 +315,7 @@ InstructionTLBMiss:
>>   	EXCEPTION_PROLOG_0
>>   	mtspr	SPRN_SPRG_SCRATCH2, r10
>>   	mfspr	r10, SPRN_SRR0	/* Get effective address of fault */
>> -#ifdef CONFIG_8xx_CPU15
>> -	addi	r11, r10, PAGE_SIZE
>> -	tlbie	r11
>> -	addi	r11, r10, -PAGE_SIZE
>> -	tlbie	r11
>> -#endif
>> +	DO_8xx_CPU15(r11, r10)
>>
>>   	/* If we are faulting a kernel address, we have to use the
>>   	 * kernel page tables.
>> --
>> 2.1.0
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[David Laight]

RnJvbTogbGVyb3kgDQo+IExlIDIwLzAxLzIwMTUgMTI6MDksIERhdmlkIExhaWdodCBhIMOpY3Jp
dCA6DQo+ID4gIEZyb20gQ2hyaXN0b3BoZSBMZXJveQ0KPiA+PiBIYXZpbmcgYSBtYWNybyB3aWxs
IGhlbHAga2VlcCBjbGVhciBjb2RlLg0KPiA+IEl0IG1pZ2h0IHJlbW92ZSBhbiAjaWYgYnV0IGl0
IGRvZXNuJ3QgcmVhbGx5IGhlbHAuDQo+ID4gQWxsIGl0IG1lYW5zIGlzIHRoYXQgYW55b25lIHJl
YWRpbmcgdGhlIGNvZGUgaGFzIHRvIGh1bnQgZm9yDQo+ID4gdGhlIGRlZmluaXRpb24gYmVmb3Jl
IHByb2NlZWRpbmcuDQo+ID4NCj4gPiBTb21lIGNvbW1lbnQgYWJvdXQgd2hhdCAoYW5kIHdoeSkg
dGhlIGV4dHJhIGNvZGUgaXMgbmVlZGVkDQo+ID4gbWlnaHQgaGVscC4NCj4gVGhlIG1haW4gcmVh
c29uIGlzIGJlY2F1c2Ugb2YgcGF0Y2ggMDkvMTEgd2hlcmUgd2UgaGF2ZSB0byBkdXBsaWNhdGUN
Cj4gdGhpcyBjb2RlLiBJIHByZWZlciB0byBqdXN0IGR1cGxpY2F0ZSBvbmUgbGluZSByYXRoZXIg
dGhhbiBkdXBsaWNhdGUgdGhlDQo+IHdob2xlIGNvZGUgKGVzcGVjaWFsbHkgYmVjYXVzZSBpbiB2
MSBvZiB0aGUgUEFUQ0hzZXQsIGl0IHdhcyBkdXBsaWNhdGVkDQo+IHR3aWNlKToNCj4gDQo+IC0g
ICAgRE9fOHh4X0NQVTE1KHIxMSwgcjEwKQ0KPiBbLi4uXQ0KPiAjaWZkZWYgQ09ORklHX01PRFVM
RVMNCj4gWy4uLl0NCj4gKyAgICBET184eHhfQ1BVMTUocjEwLCByMTEpDQo+IFsuLi5dDQo+ICsj
ZWxzZQ0KPiArICAgIG1mc3ByICAgIHIxMCwgU1BSTl9TUlIwICAgIC8qIEdldCBlZmZlY3RpdmUg
YWRkcmVzcyBvZiBmYXVsdCAqLw0KPiArICAgIERPXzh4eF9DUFUxNShyMTEsIHIxMCkNCj4gDQo+
IElzIHRoaXMgYXBwcm9hY2ggd3JvbmcgPw0KDQpJJ2QgY2FsbCBpdCBzb21ldGhpbmcgdGhhdCBp
bmZlcnMgJ2ludmFsaWRhdGUgYWRqYWNlbnQgcGFnZXMnDQphbmQgdGhlbiBtZW50aW9uIHRoYXQg
dGhpcyBpcyBuZWVkZWQgZHVlIHRvIGEgY3B1IGVycmF0YS4NCg0KCURhdmlkDQoNCg==

RE: [PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata

[David Laight]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1146 bytes --]

From: leroy 
> Le 20/01/2015 12:09, David Laight a écrit :
> >  From Christophe Leroy
> >> Having a macro will help keep clear code.
> > It might remove an #if but it doesn't really help.
> > All it means is that anyone reading the code has to hunt for
> > the definition before proceeding.
> >
> > Some comment about what (and why) the extra code is needed
> > might help.
> The main reason is because of patch 09/11 where we have to duplicate
> this code. I prefer to just duplicate one line rather than duplicate the
> whole code (especially because in v1 of the PATCHset, it was duplicated
> twice):
> 
> -    DO_8xx_CPU15(r11, r10)
> [...]
> #ifdef CONFIG_MODULES
> [...]
> +    DO_8xx_CPU15(r10, r11)
> [...]
> +#else
> +    mfspr    r10, SPRN_SRR0    /* Get effective address of fault */
> +    DO_8xx_CPU15(r11, r10)
> 
> Is this approach wrong ?

I'd call it something that infers 'invalidate adjacent pages'
and then mention that this is needed due to a cpu errata.

	David

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥