[PATCH] pte_fsl_booke: fix instruction TLB error permission check

[PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Li Yang]

Check the user/supervisor execution permission base on the code address.
This fixes the following oops on module loading or removing.

Unable to handle kernel paging request for instruction fetch
Faulting instruction address: 0xf938d040
Oops: Kernel access of bad area, sig: 11 [#1]

Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Jin Qing <b24347@freescale.com>
---
 arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 78aac7b..61b6687 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -684,6 +684,13 @@ interrupt_base:
 	rlwinm	r12,r12,0,16,1
 	mtspr	SPRN_MAS1,r12
 
+	/* Make up the required permissions for kernel code */
+#ifdef CONFIG_PTE_64BIT
+	li	r13,_PAGE_PRESENT | _PAGE_BAP_SX
+	oris	r13,r13,_PAGE_ACCESSED@h
+#else
+	li	r13,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
+#endif
 	b	4f
 
 	/* Get the PGD for the current thread */
@@ -691,15 +698,15 @@ interrupt_base:
 	mfspr	r11,SPRN_SPRG_THREAD
 	lwz	r11,PGDIR(r11)
 
-4:
-	/* Make up the required permissions */
+	/* Make up the required permissions for user code */
 #ifdef CONFIG_PTE_64BIT
-	li	r13,_PAGE_PRESENT | _PAGE_EXEC
+	li	r13,_PAGE_PRESENT | _PAGE_BAP_UX
 	oris	r13,r13,_PAGE_ACCESSED@h
 #else
 	li	r13,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
 #endif
 
+4:
 	FIND_PTE
 	andc.	r13,r13,r11		/* Check permission */
 
-- 
1.6.6-rc1.GIT

Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Kumar Gala]

On May 7, 2010, at 3:38 AM, Li Yang wrote:

> Check the user/supervisor execution permission base on the code =
address.
> This fixes the following oops on module loading or removing.
>=20
> Unable to handle kernel paging request for instruction fetch
> Faulting instruction address: 0xf938d040
> Oops: Kernel access of bad area, sig: 11 [#1]
>=20
> Signed-off-by: Li Yang <leoli@freescale.com>
> Signed-off-by: Jin Qing <b24347@freescale.com>
> ---
> arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)

applied to merge.

I've updated the commit message to be a bit more clear on why we need to =
do this.

Thanks for figuring this out.

- k=

Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Benjamin Herrenschmidt]

On Thu, 2010-05-13 at 14:14 -0500, Kumar Gala wrote:
> On May 7, 2010, at 3:38 AM, Li Yang wrote:
> 
> > Check the user/supervisor execution permission base on the code address.
> > This fixes the following oops on module loading or removing.
> > 
> > Unable to handle kernel paging request for instruction fetch
> > Faulting instruction address: 0xf938d040
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > 
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Jin Qing <b24347@freescale.com>
> > ---
> > arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
> > 1 files changed, 10 insertions(+), 3 deletions(-)
> 
> applied to merge.
> 
> I've updated the commit message to be a bit more clear on why we need to do this.

Not looking at the code right now ... but do we have the same issue on
64e ?

Cheers,
Ben.

RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Liu Dave-R63238]

> Not looking at the code right now ... but do we have the same=20
> issue on 64e ?

Aaron pointed the issue on FSL BookE.
http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079738.html

RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Benjamin Herrenschmidt]

On Fri, 2010-05-14 at 10:16 +0800, Liu Dave-R63238 wrote:
> > Not looking at the code right now ... but do we have the same 
> > issue on 64e ?
> 
> Aaron pointed the issue on FSL BookE.
> http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079738.html

Right, but I think 64-bit BookE has the same issue. We can fix that
later tho.

Cheers
Ben

RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Liu Dave-R63238]

> > Aaron pointed the issue on FSL BookE.
> >=20
> http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079738.htm
> > l
>=20
> Right, but I think 64-bit BookE has the same issue. We can=20
> fix that later tho.

Ben,

We also find the _PAGE_USER is different between pte-book3e and old
fsl-booke
It is UR in old fsl-booke, and is _PAGE_BAP_UR | _PAGE_BAP_SR in
pte-book3e,
which will cause the ioremap_flags issue.

Did you have a test your commit
[ea3cc330ac0cd521ff07c7cd432a1848c19a7e92]?

Thanks, Dave

RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Benjamin Herrenschmidt]

On Fri, 2010-05-14 at 17:14 +0800, Liu Dave-R63238 wrote:
> > > Aaron pointed the issue on FSL BookE.
> > > 
> > http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079738.htm
> > > l
> > 
> > Right, but I think 64-bit BookE has the same issue. We can 
> > fix that later tho.
> 
> Ben,
> 
> We also find the _PAGE_USER is different between pte-book3e and old
> fsl-booke
> It is UR in old fsl-booke, and is _PAGE_BAP_UR | _PAGE_BAP_SR in
> pte-book3e,
> which will cause the ioremap_flags issue.
> 
> Did you have a test your commit
> [ea3cc330ac0cd521ff07c7cd432a1848c19a7e92]?

I tested on some things :-) I keep asking Kumar to send me some e500
gear so I can actually test on it :-)

Cheers,
Ben.

RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Liu Dave-R63238]

> I've updated the commit message to be a bit more clear on why=20
> we need to do this.

I'm curious why the _PAGE_EXEC have different definition in pte-book3e.h
and pte-fsl-booke.h?

It is UX permission in pte-book3e, but is SX permission in
pte-fsl-booke.h.

Thanks, Dave

RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Benjamin Herrenschmidt]

On Fri, 2010-05-14 at 08:53 +0800, Liu Dave-R63238 wrote:
> > I've updated the commit message to be a bit more clear on why 
> > we need to do this.
> 
> I'm curious why the _PAGE_EXEC have different definition in pte-book3e.h
> and pte-fsl-booke.h?
> 
> It is UX permission in pte-book3e, but is SX permission in
> pte-fsl-booke.h.

Oh well, there's a whole history here :-)

I'm not 100% sure of the "old" fsl-booke, but pte-book3e uses UX since
that's what _PAGE_EXEC really represents from a kernel standpoint,
user-execute.

On book3e, we need to account for the HW tablewalk, which means that the
PTE can be loaded as-is by the HW. We thus don't get a chance to test
whether we are user or supervisor or change the bits accordingly. We
thus need _PAGE_EXEC to fall right into UX since that's really what the
kernel manipulates it for.

It also means that user pages will not get execute permission in
supervisor mode which is a good thing :-)

For kernel-execute, we have a specific permission, which should be
obtained via PAGE_KERNEL_EXEC in vmalloc_exec().

Cheers,
Ben.

Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Benjamin Herrenschmidt]

On Thu, 2010-05-13 at 14:14 -0500, Kumar Gala wrote:
> On May 7, 2010, at 3:38 AM, Li Yang wrote:
> 
> > Check the user/supervisor execution permission base on the code address.
> > This fixes the following oops on module loading or removing.
> > 
> > Unable to handle kernel paging request for instruction fetch
> > Faulting instruction address: 0xf938d040
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > 
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Jin Qing <b24347@freescale.com>
> > ---
> > arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
> > 1 files changed, 10 insertions(+), 3 deletions(-)
> 
> applied to merge.
> 
> I've updated the commit message to be a bit more clear on why we need to do this.
> 
> Thanks for figuring this out.

Should I pull & send to Linus ?

Cheers,
Ben.

Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check

[Kumar Gala]

On May 13, 2010, at 10:36 PM, Benjamin Herrenschmidt wrote:

> On Thu, 2010-05-13 at 14:14 -0500, Kumar Gala wrote:
>> On May 7, 2010, at 3:38 AM, Li Yang wrote:
>>=20
>>> Check the user/supervisor execution permission base on the code =
address.
>>> This fixes the following oops on module loading or removing.
>>>=20
>>> Unable to handle kernel paging request for instruction fetch
>>> Faulting instruction address: 0xf938d040
>>> Oops: Kernel access of bad area, sig: 11 [#1]
>>>=20
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> Signed-off-by: Jin Qing <b24347@freescale.com>
>>> ---
>>> arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
>>> 1 files changed, 10 insertions(+), 3 deletions(-)
>>=20
>> applied to merge.
>>=20
>> I've updated the commit message to be a bit more clear on why we need =
to do this.
>>=20
>> Thanks for figuring this out.
>=20
> Should I pull & send to Linus ?

No, I've got another patch to fix a FTRACE issue with SMP CPU bringup.  =
Will post a tree for you here shortly.

- k=