Patchwork [lucid/fsl-imx51] ARM: 5746/1: Handle possible translation errors in ARMv6/v7 coherent_user_range

login
register
mail settings
Submitter Jeremy Kerr
Date March 8, 2011, 2:03 a.m.
Message ID <1299549812.50179.618988780375.1.gpush@pororo>
Download mbox | patch
Permalink /patch/85879/
State New
Headers show

Comments

Jeremy Kerr - March 8, 2011, 2:03 a.m.
From: Catalin Marinas <catalin.marinas@arm.com>

BugLink: http://launchpad.net/bugs/605042

This is needed because applications using the sys_cacheflush system call
can pass a memory range which isn't mapped yet even though the
corresponding vma is valid. The patch also adds unwinding annotations
for correct backtraces from the coherent_user_range() functions.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

cherry-picked from upstream commit 32cfb1b16f2b68d2296536811cadfffe26a06c1b

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
 arch/arm/mm/cache-v6.S |   20 ++++++++++++++++++--
 arch/arm/mm/cache-v7.S |   19 +++++++++++++++++--
 2 files changed, 35 insertions(+), 4 deletions(-)
Bryan Wu - March 8, 2011, 2:20 a.m.
On Tue, Mar 8, 2011 at 10:03 AM, Jeremy Kerr <jeremy.kerr@canonical.com> wrote:
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> BugLink: http://launchpad.net/bugs/605042
>
> This is needed because applications using the sys_cacheflush system call
> can pass a memory range which isn't mapped yet even though the
> corresponding vma is valid. The patch also adds unwinding annotations
> for correct backtraces from the coherent_user_range() functions.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>
> cherry-picked from upstream commit 32cfb1b16f2b68d2296536811cadfffe26a06c1b
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>

That's great, it looks like it fixed an very old issue for fsl-imx51 in Lucid.
So I think this patch is for [Lucid] [fsl-imx51] and there is no such
issue in Maverick kernel, right?

-Bryan

> ---
>  arch/arm/mm/cache-v6.S |   20 ++++++++++++++++++--
>  arch/arm/mm/cache-v7.S |   19 +++++++++++++++++--
>  2 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
> index 8f5c13f..295e25d 100644
> --- a/arch/arm/mm/cache-v6.S
> +++ b/arch/arm/mm/cache-v6.S
> @@ -12,6 +12,7 @@
>  #include <linux/linkage.h>
>  #include <linux/init.h>
>  #include <asm/assembler.h>
> +#include <asm/unwind.h>
>
>  #include "proc-macros.S"
>
> @@ -121,11 +122,13 @@ ENTRY(v6_coherent_kern_range)
>  *     - the Icache does not read data from the write buffer
>  */
>  ENTRY(v6_coherent_user_range)
> -
> + UNWIND(.fnstart               )
>  #ifdef HARVARD_CACHE
>        bic     r0, r0, #CACHE_LINE_SIZE - 1
> -1:     mcr     p15, 0, r0, c7, c10, 1          @ clean D line
> +1:
> + USER( mcr     p15, 0, r0, c7, c10, 1  )       @ clean D line
>        add     r0, r0, #CACHE_LINE_SIZE
> +2:
>        cmp     r0, r1
>        blo     1b
>  #endif
> @@ -143,6 +146,19 @@ ENTRY(v6_coherent_user_range)
>        mov     pc, lr
>
>  /*
> + * Fault handling for the cache operation above. If the virtual address in r0
> + * isn't mapped, just try the next page.
> + */
> +9001:
> +       mov     r0, r0, lsr #12
> +       mov     r0, r0, lsl #12
> +       add     r0, r0, #4096
> +       b       2b
> + UNWIND(.fnend         )
> +ENDPROC(v6_coherent_user_range)
> +ENDPROC(v6_coherent_kern_range)
> +
> +/*
>  *     v6_flush_kern_dcache_page(kaddr)
>  *
>  *     Ensure that the data held in the page kaddr is written back
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index be93ff0..3290dac 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -13,6 +13,7 @@
>  #include <linux/linkage.h>
>  #include <linux/init.h>
>  #include <asm/assembler.h>
> +#include <asm/unwind.h>
>
>  #include "proc-macros.S"
>
> @@ -147,13 +148,16 @@ ENTRY(v7_coherent_kern_range)
>  *     - the Icache does not read data from the write buffer
>  */
>  ENTRY(v7_coherent_user_range)
> + UNWIND(.fnstart               )
>        dcache_line_size r2, r3
>        sub     r3, r2, #1
>        bic     r0, r0, r3
> -1:     mcr     p15, 0, r0, c7, c11, 1          @ clean D line to the point of unification
> +1:
> + USER( mcr     p15, 0, r0, c7, c11, 1  )       @ clean D line to the point of unification
>        dsb
> -       mcr     p15, 0, r0, c7, c5, 1           @ invalidate I line
> + USER( mcr     p15, 0, r0, c7, c5, 1   )       @ invalidate I line
>        add     r0, r0, r2
> +2:
>        cmp     r0, r1
>        blo     1b
>        mov     r0, #0
> @@ -161,6 +165,17 @@ ENTRY(v7_coherent_user_range)
>        dsb
>        isb
>        mov     pc, lr
> +
> +/*
> + * Fault handling for the cache operation above. If the virtual address in r0
> + * isn't mapped, just try the next page.
> + */
> +9001:
> +       mov     r0, r0, lsr #12
> +       mov     r0, r0, lsl #12
> +       add     r0, r0, #4096
> +       b       2b
> + UNWIND(.fnend         )
>  ENDPROC(v7_coherent_kern_range)
>  ENDPROC(v7_coherent_user_range)
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Jeremy Kerr - March 8, 2011, 2:29 a.m.
Hi Bryan,

> That's great, it looks like it fixed an very old issue for fsl-imx51 in
> Lucid.

It's only shown up with relatively recent versions of eglibc - it seems like 
they added a call to cacheflush() in the mprotect() syscall wrapper.

> So I think this patch is for [Lucid] [fsl-imx51] and there is no
> such issue in Maverick kernel, right?

Yeah, this change went upstream prior to the maverick base, so anything post-
lucid already has this fix.

Cheers,


Jeremy
Tim Gardner - March 8, 2011, 1:51 p.m.
On 03/08/2011 02:03 AM, Jeremy Kerr wrote:
> From: Catalin Marinas<catalin.marinas@arm.com>
>
> BugLink: http://launchpad.net/bugs/605042
>
> This is needed because applications using the sys_cacheflush system call
> can pass a memory range which isn't mapped yet even though the
> corresponding vma is valid. The patch also adds unwinding annotations
> for correct backtraces from the coherent_user_range() functions.
>
> Signed-off-by: Catalin Marinas<catalin.marinas@arm.com>
> Signed-off-by: Russell King<rmk+kernel@arm.linux.org.uk>
>
> cherry-picked from upstream commit 32cfb1b16f2b68d2296536811cadfffe26a06c1b
>
> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
>

Applied and uploaded linux-fsl-imx51_2.6.31-608.23 to 
https://launchpad.net/~canonical-kernel-team/+archive/ppa

rtg
Matthias Klose - March 8, 2011, 10:18 p.m.
[CCing Lamont and Colin]

On 08.03.2011 14:51, Tim Gardner wrote:
> On 03/08/2011 02:03 AM, Jeremy Kerr wrote:
>> From: Catalin Marinas<catalin.marinas@arm.com>
>>
>> BugLink: http://launchpad.net/bugs/605042
>>
>> This is needed because applications using the sys_cacheflush system call
>> can pass a memory range which isn't mapped yet even though the
>> corresponding vma is valid. The patch also adds unwinding annotations
>> for correct backtraces from the coherent_user_range() functions.
>>
>> Signed-off-by: Catalin Marinas<catalin.marinas@arm.com>
>> Signed-off-by: Russell King<rmk+kernel@arm.linux.org.uk>
>>
>> cherry-picked from upstream commit 32cfb1b16f2b68d2296536811cadfffe26a06c1b
>>
>> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
>>
> 
> Applied and uploaded linux-fsl-imx51_2.6.31-608.23 to
> https://launchpad.net/~canonical-kernel-team/+archive/ppa

II: Checking ABI for imx51...
EE: Previous or current ABI file missing!

/build/buildd/linux-fsl-imx51-2.6.31/debian.fsl-imx51/abi/2.6.31-608.22/armel/imx51
make[1]: *** [abi-check-imx51] Error 1
make: *** [binary-arch] Error 2

[Colin, please could you revert the eglibc change for natty,
 if we cannot get a fixed kernel on the buildds?]

  Matthias, mostly away for this week.
Tim Gardner - March 8, 2011, 11:03 p.m.
On 03/08/2011 10:18 PM, Matthias Klose wrote:
> [CCing Lamont and Colin]
>
> On 08.03.2011 14:51, Tim Gardner wrote:
>> On 03/08/2011 02:03 AM, Jeremy Kerr wrote:
>>> From: Catalin Marinas<catalin.marinas@arm.com>
>>>
>>> BugLink: http://launchpad.net/bugs/605042
>>>
>>> This is needed because applications using the sys_cacheflush system call
>>> can pass a memory range which isn't mapped yet even though the
>>> corresponding vma is valid. The patch also adds unwinding annotations
>>> for correct backtraces from the coherent_user_range() functions.
>>>
>>> Signed-off-by: Catalin Marinas<catalin.marinas@arm.com>
>>> Signed-off-by: Russell King<rmk+kernel@arm.linux.org.uk>
>>>
>>> cherry-picked from upstream commit 32cfb1b16f2b68d2296536811cadfffe26a06c1b
>>>
>>> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
>>>
>> Applied and uploaded linux-fsl-imx51_2.6.31-608.23 to
>> https://launchpad.net/~canonical-kernel-team/+archive/ppa
> II: Checking ABI for imx51...
> EE: Previous or current ABI file missing!
>
> /build/buildd/linux-fsl-imx51-2.6.31/debian.fsl-imx51/abi/2.6.31-608.22/armel/imx51
> make[1]: *** [abi-check-imx51] Error 1
> make: *** [binary-arch] Error 2
>
> [Colin, please could you revert the eglibc change for natty,
>   if we cannot get a fixed kernel on the buildds?]
>
>    Matthias, mostly away for this week.

Chill dude, I'll get it fixed tomorrow.
Matthias Klose - March 8, 2011, 11:15 p.m.
On 09.03.2011 00:03, Tim Gardner wrote:
> On 03/08/2011 10:18 PM, Matthias Klose wrote:
>> [CCing Lamont and Colin]
>>
>> On 08.03.2011 14:51, Tim Gardner wrote:
>>> On 03/08/2011 02:03 AM, Jeremy Kerr wrote:
>>>> From: Catalin Marinas<catalin.marinas@arm.com>
>>>>
>>>> BugLink: http://launchpad.net/bugs/605042
>>>>
>>>> This is needed because applications using the sys_cacheflush system call
>>>> can pass a memory range which isn't mapped yet even though the
>>>> corresponding vma is valid. The patch also adds unwinding annotations
>>>> for correct backtraces from the coherent_user_range() functions.
>>>>
>>>> Signed-off-by: Catalin Marinas<catalin.marinas@arm.com>
>>>> Signed-off-by: Russell King<rmk+kernel@arm.linux.org.uk>
>>>>
>>>> cherry-picked from upstream commit 32cfb1b16f2b68d2296536811cadfffe26a06c1b
>>>>
>>>> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
>>>>
>>> Applied and uploaded linux-fsl-imx51_2.6.31-608.23 to
>>> https://launchpad.net/~canonical-kernel-team/+archive/ppa
>> II: Checking ABI for imx51...
>> EE: Previous or current ABI file missing!
>>
>> /build/buildd/linux-fsl-imx51-2.6.31/debian.fsl-imx51/abi/2.6.31-608.22/armel/imx51
>>
>> make[1]: *** [abi-check-imx51] Error 1
>> make: *** [binary-arch] Error 2
>>
>> [Colin, please could you revert the eglibc change for natty,
>>   if we cannot get a fixed kernel on the buildds?]
>>
>>    Matthias, mostly away for this week.
> 
> Chill dude, I'll get it fixed tomorrow.

sorry, couldn't reach you on irc.

if all members of the kernel team would chill like this ...

  Matthias

Patch

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 8f5c13f..295e25d 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -12,6 +12,7 @@ 
 #include <linux/linkage.h>
 #include <linux/init.h>
 #include <asm/assembler.h>
+#include <asm/unwind.h>
 
 #include "proc-macros.S"
 
@@ -121,11 +122,13 @@  ENTRY(v6_coherent_kern_range)
  *	- the Icache does not read data from the write buffer
  */
 ENTRY(v6_coherent_user_range)
-
+ UNWIND(.fnstart		)
 #ifdef HARVARD_CACHE
 	bic	r0, r0, #CACHE_LINE_SIZE - 1
-1:	mcr	p15, 0, r0, c7, c10, 1		@ clean D line
+1:
+ USER(	mcr	p15, 0, r0, c7, c10, 1	)	@ clean D line
 	add	r0, r0, #CACHE_LINE_SIZE
+2:
 	cmp	r0, r1
 	blo	1b
 #endif
@@ -143,6 +146,19 @@  ENTRY(v6_coherent_user_range)
 	mov	pc, lr
 
 /*
+ * Fault handling for the cache operation above. If the virtual address in r0
+ * isn't mapped, just try the next page.
+ */
+9001:
+	mov	r0, r0, lsr #12
+	mov	r0, r0, lsl #12
+	add	r0, r0, #4096
+	b	2b
+ UNWIND(.fnend		)
+ENDPROC(v6_coherent_user_range)
+ENDPROC(v6_coherent_kern_range)
+
+/*
  *	v6_flush_kern_dcache_page(kaddr)
  *
  *	Ensure that the data held in the page kaddr is written back
diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index be93ff0..3290dac 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -13,6 +13,7 @@ 
 #include <linux/linkage.h>
 #include <linux/init.h>
 #include <asm/assembler.h>
+#include <asm/unwind.h>
 
 #include "proc-macros.S"
 
@@ -147,13 +148,16 @@  ENTRY(v7_coherent_kern_range)
  *	- the Icache does not read data from the write buffer
  */
 ENTRY(v7_coherent_user_range)
+ UNWIND(.fnstart		)
 	dcache_line_size r2, r3
 	sub	r3, r2, #1
 	bic	r0, r0, r3
-1:	mcr	p15, 0, r0, c7, c11, 1		@ clean D line to the point of unification
+1:
+ USER(	mcr	p15, 0, r0, c7, c11, 1	)	@ clean D line to the point of unification
 	dsb
-	mcr	p15, 0, r0, c7, c5, 1		@ invalidate I line
+ USER(	mcr	p15, 0, r0, c7, c5, 1	)	@ invalidate I line
 	add	r0, r0, r2
+2:
 	cmp	r0, r1
 	blo	1b
 	mov	r0, #0
@@ -161,6 +165,17 @@  ENTRY(v7_coherent_user_range)
 	dsb
 	isb
 	mov	pc, lr
+
+/*
+ * Fault handling for the cache operation above. If the virtual address in r0
+ * isn't mapped, just try the next page.
+ */
+9001:
+	mov	r0, r0, lsr #12
+	mov	r0, r0, lsl #12
+	add	r0, r0, #4096
+	b	2b
+ UNWIND(.fnend		)
 ENDPROC(v7_coherent_kern_range)
 ENDPROC(v7_coherent_user_range)