Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Aaro Koskinen]

Hello,

ext Kirill A. Shutemov wrote:
>  # ifdef CPU_PABORT_HANDLER
>  #  define MULTI_PABORT 1
>  # else
> -#  define CPU_PABORT_HANDLER(reg, insn)	mrc p15, 0, reg, cr6, cr0, 2
> +#  define CPU_PABORT_HANDLER ifar_pabort
>  # endif
>  #endif
>  
> @@ -138,7 +138,7 @@
>  # ifdef CPU_PABORT_HANDLER
>  #  define MULTI_PABORT 1
>  # else
> -#  define CPU_PABORT_HANDLER(reg, insn)	mov reg, insn
> +#  define CPU_PABORT_HANDLER noifar_pabort
>  # endif
>  #endif

I think the point of these was that they can be inlined. Note that the 
kernel already have functions pabort_ifar() and pabort_noifar(). You 
should modifed them _and_ the inlined asm.

> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 85040cf..dbb4ce7 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -291,22 +291,16 @@ __pabt_svc:
>  	tst	r3, #PSR_I_BIT
>  	biceq	r9, r9, #PSR_I_BIT
>  
> -	@
> -	@ set args, then call main handler
> -	@
> -	@  r0 - address of faulting instruction
> -	@  r1 - pointer to registers on stack
> -	@
>  #ifdef MULTI_PABORT
>  	mov	r0, r2			@ pass address of aborted instruction.
>  	ldr	r4, .LCprocfns
>  	mov	lr, pc
>  	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
>  #else
> -	CPU_PABORT_HANDLER(r0, r2)
> +	bl	CPU_PABORT_HANDLER

You are not passing the correct parameter in r0.

>  #endif
>  	msr	cpsr_c, r9			@ Maybe enable interrupts
> -	mov	r1, sp				@ regs
> +	mov	r2, sp				@ regs
>  	bl	do_PrefetchAbort		@ call abort handler
>  
>  	@
> @@ -666,10 +660,10 @@ __pabt_usr:
>  	mov	lr, pc
>  	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
>  #else
> -	CPU_PABORT_HANDLER(r0, r2)
> +	bl	CPU_PABORT_HANDLER

Same here.

>  #endif
>  	enable_irq				@ Enable interrupts
> -	mov	r1, sp				@ regs
> +	mov	r2, sp				@ regs
>  	bl	do_PrefetchAbort		@ call abort handler
>  	/* fall through */
>  /*
> diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
> index 480f78a..30f1708 100644
> --- a/arch/arm/mm/Makefile
> +++ b/arch/arm/mm/Makefile
> @@ -78,3 +78,5 @@ obj-$(CONFIG_CACHE_FEROCEON_L2)	+= cache-feroceon-l2.o
>  obj-$(CONFIG_CACHE_L2X0)	+= cache-l2x0.o
>  obj-$(CONFIG_CACHE_XSC3L2)	+= cache-xsc3l2.o
>  
> +obj-$(CONFIG_CPU_PABRT_NOIFAR)	+= pabort-noifar.o
> +obj-$(CONFIG_CPU_PABRT_IFAR)	+= pabort-ifar.o

These are duplicate functions.

> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index 22c9530..c7bbb32 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -480,7 +480,7 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  }
>  
>  asmlinkage void __exception
> -do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
> +do_PrefetchAbort(unsigned long addr, int ifsr, struct pt_regs *regs)
>  {
>  	do_translation_fault(addr, 0, regs);
>  }
> diff --git a/arch/arm/mm/pabort-ifar.S b/arch/arm/mm/pabort-ifar.S
> new file mode 100644
> index 0000000..46838ea
> --- /dev/null
> +++ b/arch/arm/mm/pabort-ifar.S
> @@ -0,0 +1,18 @@
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +
> +/*
> + * Function: ifar_pabort
> + *
> + * Returns : r0 = address of abort
> + *	   : r1 = IFSR
> + *
> + * Purpose : obtain information about current prefetch abort.
> + */
> +	.align	5
> +ENTRY(ifar_pabort)
> +	mrc	p15, 0, r0, c6, c0, 2		@ get IFAR
> +	mrc	p15, 0, r1, c5, c0, 1		@ get IFSR
> +
> +	mov	pc, lr
> +ENDPROC(ifar_pabort)
> diff --git a/arch/arm/mm/pabort-noifar.S b/arch/arm/mm/pabort-noifar.S
> new file mode 100644
> index 0000000..46b0daf
> --- /dev/null
> +++ b/arch/arm/mm/pabort-noifar.S
> @@ -0,0 +1,18 @@
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +
> +/*
> + * Function: noifar_pabort
> +
> + * Returns : r0 = address of abort
> + *	   : r1 = 5, simulate IFSR with section translation fault status
> + *
> + * Purpose : obtain information about current prefetch abort.
> + */
> +	.align	5
> +ENTRY(noifar_pabort)
> +	mov	r0, insn
> +	mov	r1, #5		@ translation fault
> +
> +	mov	pc, lr
> +ENDPROC(noifar_pabort)

What's the "insn" here?? And even if no IFAR is implemented, there 
should be IFSR. For NOMMU there should be pabort_noifsr simply returning 
zero values, similar to data abort handler.

A.

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Aaro Koskinen]

On Fri, 18 Sep 2009, ext Kirill A. Shutemov wrote:

> It needed for proper prefetch abort handling on ARMv7.

Here's what I had in mind:

---
  arch/arm/include/asm/glue.h    |   24 ++++++++++++++++++------
  arch/arm/kernel/entry-armv.S   |   10 ++++++----
  arch/arm/kernel/entry-common.S |    1 +
  arch/arm/mm/Kconfig            |    9 ++++++---
  arch/arm/mm/Makefile           |    2 ++
  arch/arm/mm/fault.c            |   20 ++++++++++++++++++--
  arch/arm/mm/pabort-noifsr.S    |   16 ++++++++++++++++
  arch/arm/mm/proc-arm940.S      |    2 +-
  arch/arm/mm/proc-arm946.S      |    2 +-
  arch/arm/mm/proc-arm9tdmi.S    |    2 +-
  10 files changed, 70 insertions(+), 18 deletions(-)
  create mode 100644 arch/arm/mm/pabort-noifsr.S

diff --git a/arch/arm/include/asm/glue.h b/arch/arm/include/asm/glue.h
index a0e39d5..1427fef 100644
--- a/arch/arm/include/asm/glue.h
+++ b/arch/arm/include/asm/glue.h
@@ -123,26 +123,38 @@
   * Prefetch abort handler.  If the CPU has an IFAR use that, otherwise
   * use the address of the aborted instruction
   */
-#undef CPU_PABORT_HANDLER
+#undef CPU_PABORT_HANDLER_IFAR
+#undef CPU_PABORT_HANDLER_IFSR
  #undef MULTI_PABORT

  #ifdef CONFIG_CPU_PABRT_IFAR
-# ifdef CPU_PABORT_HANDLER
+# ifdef CPU_PABORT_HANDLER_IFAR
  #  define MULTI_PABORT 1
  # else
-#  define CPU_PABORT_HANDLER(reg, insn)	mrc p15, 0, reg, cr6, cr0, 2
+#  define CPU_PABORT_HANDLER_IFAR(reg, insn)	mrc p15, 0, reg, cr6, cr0, 2
+#  define CPU_PABORT_HANDLER_IFSR(reg)		mrc p15, 0, reg, cr5, cr0, 1
  # endif
  #endif

  #ifdef CONFIG_CPU_PABRT_NOIFAR
-# ifdef CPU_PABORT_HANDLER
+# ifdef CPU_PABORT_HANDLER_IFAR
  #  define MULTI_PABORT 1
  # else
-#  define CPU_PABORT_HANDLER(reg, insn)	mov reg, insn
+#  define CPU_PABORT_HANDLER_IFAR(reg, insn)	mov reg, insn
+#  define CPU_PABORT_HANDLER_IFSR(reg)		mrc p15, 0, reg, cr5, cr0, 1
  # endif
  #endif

-#ifndef CPU_PABORT_HANDLER
+#ifdef CONFIG_CPU_PABRT_NOIFSR
+# ifdef CPU_PABORT_HANDLER_IFAR
+#  define MULTI_PABORT 1
+# else
+#  define CPU_PABORT_HANDLER_IFAR(reg, insn)	mov reg, insn
+#  define CPU_PABORT_HANDLER_IFSR(reg)		mov reg, #0
+# endif
+#endif
+
+#if !defined(CPU_PABORT_HANDLER_IFAR) || !defined(CPU_PABORT_HANDLER_IFSR)
  #error Unknown prefetch abort handler type
  #endif

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 3d727a8..e252d32 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -315,10 +315,11 @@ __pabt_svc:
  	mov	lr, pc
  	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
  #else
-	CPU_PABORT_HANDLER(r0, r2)
+	CPU_PABORT_HANDLER_IFAR(r0, r2)
+	CPU_PABORT_HANDLER_IFSR(r1)
  #endif
  	msr	cpsr_c, r9			@ Maybe enable interrupts
-	mov	r1, sp				@ regs
+	mov	r2, sp				@ regs
  	bl	do_PrefetchAbort		@ call abort handler

  	@
@@ -697,10 +698,11 @@ __pabt_usr:
  	mov	lr, pc
  	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
  #else
-	CPU_PABORT_HANDLER(r0, r2)
+	CPU_PABORT_HANDLER_IFAR(r0, r2)
+	CPU_PABORT_HANDLER_IFSR(r1)
  #endif
  	enable_irq				@ Enable interrupts
-	mov	r1, sp				@ regs
+	mov	r2, sp				@ regs
  	bl	do_PrefetchAbort		@ call abort handler
   UNWIND(.fnend		)
  	/* fall through */
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 807cfeb..254465d 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -428,6 +428,7 @@ ENDPROC(sys_mmap2)
  ENTRY(pabort_ifar)
  		mrc	p15, 0, r0, cr6, cr0, 2
  ENTRY(pabort_noifar)
+		mrc     p15, 0, r1, cr5, cr0, 1           @ fsr
  		mov	pc, lr
  ENDPROC(pabort_ifar)
  ENDPROC(pabort_noifar)
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 5fe595a..d0ee034 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -100,7 +100,7 @@ config CPU_ARM9TDMI
  	depends on !MMU
  	select CPU_32v4T
  	select CPU_ABRT_NOMMU
-	select CPU_PABRT_NOIFAR
+	select CPU_PABRT_NOIFSR
  	select CPU_CACHE_V4
  	help
  	  A 32-bit RISC microprocessor based on the ARM9 processor core
@@ -210,7 +210,7 @@ config CPU_ARM940T
  	depends on !MMU
  	select CPU_32v4T
  	select CPU_ABRT_NOMMU
-	select CPU_PABRT_NOIFAR
+	select CPU_PABRT_NOIFSR
  	select CPU_CACHE_VIVT
  	select CPU_CP15_MPU
  	help
@@ -228,7 +228,7 @@ config CPU_ARM946E
  	depends on !MMU
  	select CPU_32v5
  	select CPU_ABRT_NOMMU
-	select CPU_PABRT_NOIFAR
+	select CPU_PABRT_NOIFSR
  	select CPU_CACHE_VIVT
  	select CPU_CP15_MPU
  	help
@@ -488,6 +488,9 @@ config CPU_PABRT_IFAR
  config CPU_PABRT_NOIFAR
  	bool

+config CPU_PABRT_NOIFSR
+	bool
+
  # The cache model
  config CPU_CACHE_V3
  	bool
diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index 63e3f6d..5c1062d 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -27,6 +27,8 @@ obj-$(CONFIG_CPU_ABRT_EV5TJ)	+= abort-ev5tj.o
  obj-$(CONFIG_CPU_ABRT_EV6)	+= abort-ev6.o
  obj-$(CONFIG_CPU_ABRT_EV7)	+= abort-ev7.o

+obj-$(CONFIG_CPU_PABRT_NOIFSR)	+= pabort-noifsr.o
+
  obj-$(CONFIG_CPU_CACHE_V3)	+= cache-v3.o
  obj-$(CONFIG_CPU_CACHE_V4)	+= cache-v4.o
  obj-$(CONFIG_CPU_CACHE_V4WT)	+= cache-v4wt.o
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index cc8829d..70fdd66 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -506,8 +506,24 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  }

  asmlinkage void __exception
-do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
+do_PrefetchAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  {
-	do_translation_fault(addr, 0, regs);
+	struct siginfo info;
+
+	switch (fsr & 15) {
+	case 5:
+	case 7:
+		do_translation_fault(addr, fsr, regs);
+		break;
+	default:
+		printk(KERN_ALERT "Prefetch abort: 0x%03x at %08lx\n",
+		       fsr, addr);
+	case 15:
+		info.si_signo = SIGSEGV;
+		info.si_errno = 0;
+		info.si_code  = SEGV_ACCERR;
+		info.si_addr  = (void __user *)addr;
+		arm_notify_die("", regs, &info, fsr, 0);
+	}
  }

diff --git a/arch/arm/mm/pabort-noifsr.S b/arch/arm/mm/pabort-noifsr.S
new file mode 100644
index 0000000..4abd848
--- /dev/null
+++ b/arch/arm/mm/pabort-noifsr.S
@@ -0,0 +1,16 @@
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+/*
+ * Function: pabort_noifsr
+ *
+ * Params  : r0 = address of aborted instruction
+ *
+ * Returns : r0 = r0 (abort address)
+ *	   : r1 = 0 (FSR)
+ *
+ */
+	.align	5
+ENTRY(pabort_noifsr)
+	mov	r1, #0
+	mov	pc, lr
+ENDPROC(pabort_noifsr)
diff --git a/arch/arm/mm/proc-arm940.S b/arch/arm/mm/proc-arm940.S
index f595117..5684581 100644
--- a/arch/arm/mm/proc-arm940.S
+++ b/arch/arm/mm/proc-arm940.S
@@ -322,7 +322,7 @@ __arm940_setup:
  	.type	arm940_processor_functions, #object
  ENTRY(arm940_processor_functions)
  	.word	nommu_early_abort
-	.word	pabort_noifar
+	.word	pabort_noifsr
  	.word	cpu_arm940_proc_init
  	.word	cpu_arm940_proc_fin
  	.word	cpu_arm940_reset
diff --git a/arch/arm/mm/proc-arm946.S b/arch/arm/mm/proc-arm946.S
index e03f6ff..f82c7fe 100644
--- a/arch/arm/mm/proc-arm946.S
+++ b/arch/arm/mm/proc-arm946.S
@@ -377,7 +377,7 @@ __arm946_setup:
  	.type	arm946_processor_functions, #object
  ENTRY(arm946_processor_functions)
  	.word	nommu_early_abort
-	.word	pabort_noifar
+	.word	pabort_noifsr
  	.word	cpu_arm946_proc_init
  	.word	cpu_arm946_proc_fin
  	.word	cpu_arm946_reset
diff --git a/arch/arm/mm/proc-arm9tdmi.S b/arch/arm/mm/proc-arm9tdmi.S
index be6c11d..db42c52 100644
--- a/arch/arm/mm/proc-arm9tdmi.S
+++ b/arch/arm/mm/proc-arm9tdmi.S
@@ -64,7 +64,7 @@ __arm9tdmi_setup:
  		.type	arm9tdmi_processor_functions, #object
  ENTRY(arm9tdmi_processor_functions)
  		.word	nommu_early_abort
-		.word	pabort_noifar
+		.word	pabort_noifsr
  		.word	cpu_arm9tdmi_proc_init
  		.word	cpu_arm9tdmi_proc_fin
  		.word	cpu_arm9tdmi_reset
-- 
1.5.4.3

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

On Fri, Sep 18, 2009 at 2:18 PM, Aaro Koskinen <aaro.koskinen@nokia.com> wrote:
> What's the "insn" here?? And even if no IFAR is implemented, there should be
> IFSR. For NOMMU there should be pabort_noifsr simply returning zero values,
> similar to data abort handler.

No, both IFAR and IFSR is only implemented in ARMv7 so we have to handle
it together, I think.

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

On Fri, Sep 18, 2009 at 2:28 PM, Aaro Koskinen <aakoskin@nokia.com> wrote:
> On Fri, 18 Sep 2009, ext Kirill A. Shutemov wrote:
>
>> It needed for proper prefetch abort handling on ARMv7.
>
> Here's what I had in mind:
>
> ---
>  arch/arm/include/asm/glue.h    |   24 ++++++++++++++++++------
>  arch/arm/kernel/entry-armv.S   |   10 ++++++----
>  arch/arm/kernel/entry-common.S |    1 +
>  arch/arm/mm/Kconfig            |    9 ++++++---
>  arch/arm/mm/Makefile           |    2 ++
>  arch/arm/mm/fault.c            |   20 ++++++++++++++++++--
>  arch/arm/mm/pabort-noifsr.S    |   16 ++++++++++++++++
>  arch/arm/mm/proc-arm940.S      |    2 +-
>  arch/arm/mm/proc-arm946.S      |    2 +-
>  arch/arm/mm/proc-arm9tdmi.S    |    2 +-
>  10 files changed, 70 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/mm/pabort-noifsr.S
>
> diff --git a/arch/arm/include/asm/glue.h b/arch/arm/include/asm/glue.h
> index a0e39d5..1427fef 100644
> --- a/arch/arm/include/asm/glue.h
> +++ b/arch/arm/include/asm/glue.h
> @@ -123,26 +123,38 @@
>  * Prefetch abort handler.  If the CPU has an IFAR use that, otherwise
>  * use the address of the aborted instruction
>  */
> -#undef CPU_PABORT_HANDLER
> +#undef CPU_PABORT_HANDLER_IFAR
> +#undef CPU_PABORT_HANDLER_IFSR
>  #undef MULTI_PABORT
>
>  #ifdef CONFIG_CPU_PABRT_IFAR
> -# ifdef CPU_PABORT_HANDLER
> +# ifdef CPU_PABORT_HANDLER_IFAR
>  #  define MULTI_PABORT 1
>  # else
> -#  define CPU_PABORT_HANDLER(reg, insn)        mrc p15, 0, reg, cr6, cr0, 2
> +#  define CPU_PABORT_HANDLER_IFAR(reg, insn)   mrc p15, 0, reg, cr6, cr0, 2
> +#  define CPU_PABORT_HANDLER_IFSR(reg)         mrc p15, 0, reg, cr5, cr0, 1
>  # endif
>  #endif
>
>  #ifdef CONFIG_CPU_PABRT_NOIFAR
> -# ifdef CPU_PABORT_HANDLER
> +# ifdef CPU_PABORT_HANDLER_IFAR
>  #  define MULTI_PABORT 1
>  # else
> -#  define CPU_PABORT_HANDLER(reg, insn)        mov reg, insn
> +#  define CPU_PABORT_HANDLER_IFAR(reg, insn)   mov reg, insn
> +#  define CPU_PABORT_HANDLER_IFSR(reg)         mrc p15, 0, reg, cr5, cr0, 1

It's incorrect. We have IFSR only on ARMv7.

>  # endif
>  #endif
>
> -#ifndef CPU_PABORT_HANDLER
> +#ifdef CONFIG_CPU_PABRT_NOIFSR
> +# ifdef CPU_PABORT_HANDLER_IFAR
> +#  define MULTI_PABORT 1
> +# else
> +#  define CPU_PABORT_HANDLER_IFAR(reg, insn)   mov reg, insn
> +#  define CPU_PABORT_HANDLER_IFSR(reg)         mov reg, #0

#0 is undefined status for IFSR, so we should to use #5 here.

> +# endif
> +#endif
> +
> +#if !defined(CPU_PABORT_HANDLER_IFAR) || !defined(CPU_PABORT_HANDLER_IFSR)
>  #error Unknown prefetch abort handler type
>  #endif
>
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 3d727a8..e252d32 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -315,10 +315,11 @@ __pabt_svc:
>        mov     lr, pc
>        ldr     pc, [r4, #PROCESSOR_PABT_FUNC]
>  #else
> -       CPU_PABORT_HANDLER(r0, r2)
> +       CPU_PABORT_HANDLER_IFAR(r0, r2)
> +       CPU_PABORT_HANDLER_IFSR(r1)
>  #endif
>        msr     cpsr_c, r9                      @ Maybe enable interrupts
> -       mov     r1, sp                          @ regs
> +       mov     r2, sp                          @ regs
>        bl      do_PrefetchAbort                @ call abort handler
>
>        @
> @@ -697,10 +698,11 @@ __pabt_usr:
>        mov     lr, pc
>        ldr     pc, [r4, #PROCESSOR_PABT_FUNC]
>  #else
> -       CPU_PABORT_HANDLER(r0, r2)
> +       CPU_PABORT_HANDLER_IFAR(r0, r2)
> +       CPU_PABORT_HANDLER_IFSR(r1)
>  #endif
>        enable_irq                              @ Enable interrupts
> -       mov     r1, sp                          @ regs
> +       mov     r2, sp                          @ regs
>        bl      do_PrefetchAbort                @ call abort handler
>  UNWIND(.fnend         )
>        /* fall through */
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 807cfeb..254465d 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -428,6 +428,7 @@ ENDPROC(sys_mmap2)
>  ENTRY(pabort_ifar)
>                mrc     p15, 0, r0, cr6, cr0, 2
>  ENTRY(pabort_noifar)
> +               mrc     p15, 0, r1, cr5, cr0, 1           @ fsr
>                mov     pc, lr
>  ENDPROC(pabort_ifar)
>  ENDPROC(pabort_noifar)
> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 5fe595a..d0ee034 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -100,7 +100,7 @@ config CPU_ARM9TDMI
>        depends on !MMU
>        select CPU_32v4T
>        select CPU_ABRT_NOMMU
> -       select CPU_PABRT_NOIFAR
> +       select CPU_PABRT_NOIFSR
>        select CPU_CACHE_V4
>        help
>          A 32-bit RISC microprocessor based on the ARM9 processor core
> @@ -210,7 +210,7 @@ config CPU_ARM940T
>        depends on !MMU
>        select CPU_32v4T
>        select CPU_ABRT_NOMMU
> -       select CPU_PABRT_NOIFAR
> +       select CPU_PABRT_NOIFSR
>        select CPU_CACHE_VIVT
>        select CPU_CP15_MPU
>        help
> @@ -228,7 +228,7 @@ config CPU_ARM946E
>        depends on !MMU
>        select CPU_32v5
>        select CPU_ABRT_NOMMU
> -       select CPU_PABRT_NOIFAR
> +       select CPU_PABRT_NOIFSR
>        select CPU_CACHE_VIVT
>        select CPU_CP15_MPU
>        help
> @@ -488,6 +488,9 @@ config CPU_PABRT_IFAR
>  config CPU_PABRT_NOIFAR
>        bool
>
> +config CPU_PABRT_NOIFSR
> +       bool
> +
>  # The cache model
>  config CPU_CACHE_V3
>        bool
> diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
> index 63e3f6d..5c1062d 100644
> --- a/arch/arm/mm/Makefile
> +++ b/arch/arm/mm/Makefile
> @@ -27,6 +27,8 @@ obj-$(CONFIG_CPU_ABRT_EV5TJ)  += abort-ev5tj.o
>  obj-$(CONFIG_CPU_ABRT_EV6)     += abort-ev6.o
>  obj-$(CONFIG_CPU_ABRT_EV7)     += abort-ev7.o
>
> +obj-$(CONFIG_CPU_PABRT_NOIFSR) += pabort-noifsr.o
> +
>  obj-$(CONFIG_CPU_CACHE_V3)     += cache-v3.o
>  obj-$(CONFIG_CPU_CACHE_V4)     += cache-v4.o
>  obj-$(CONFIG_CPU_CACHE_V4WT)   += cache-v4wt.o
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index cc8829d..70fdd66 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -506,8 +506,24 @@ do_DataAbort(unsigned long addr, unsigned int fsr,
> struct pt_regs *regs)
>  }
>
>  asmlinkage void __exception
> -do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
> +do_PrefetchAbort(unsigned long addr, unsigned int fsr, struct pt_regs
> *regs)
>  {
> -       do_translation_fault(addr, 0, regs);
> +       struct siginfo info;
> +
> +       switch (fsr & 15) {
> +       case 5:
> +       case 7:
> +               do_translation_fault(addr, fsr, regs);

I guess for 7, we should use do_page_fault instead.

> +               break;
> +       default:
> +               printk(KERN_ALERT "Prefetch abort: 0x%03x at %08lx\n",
> +                      fsr, addr);
> +       case 15:

We should also handle 13 here.

> +               info.si_signo = SIGSEGV;
> +               info.si_errno = 0;
> +               info.si_code  = SEGV_ACCERR;
> +               info.si_addr  = (void __user *)addr;
> +               arm_notify_die("", regs, &info, fsr, 0);
> +       }
>  }
>
> diff --git a/arch/arm/mm/pabort-noifsr.S b/arch/arm/mm/pabort-noifsr.S
> new file mode 100644
> index 0000000..4abd848
> --- /dev/null
> +++ b/arch/arm/mm/pabort-noifsr.S
> @@ -0,0 +1,16 @@
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +/*
> + * Function: pabort_noifsr
> + *
> + * Params  : r0 = address of aborted instruction
> + *
> + * Returns : r0 = r0 (abort address)
> + *        : r1 = 0 (FSR)
> + *
> + */
> +       .align  5
> +ENTRY(pabort_noifsr)
> +       mov     r1, #0

Use #5 instead.

> +       mov     pc, lr
> +ENDPROC(pabort_noifsr)
> diff --git a/arch/arm/mm/proc-arm940.S b/arch/arm/mm/proc-arm940.S
> index f595117..5684581 100644
> --- a/arch/arm/mm/proc-arm940.S
> +++ b/arch/arm/mm/proc-arm940.S
> @@ -322,7 +322,7 @@ __arm940_setup:
>        .type   arm940_processor_functions, #object
>  ENTRY(arm940_processor_functions)
>        .word   nommu_early_abort
> -       .word   pabort_noifar
> +       .word   pabort_noifsr
>        .word   cpu_arm940_proc_init
>        .word   cpu_arm940_proc_fin
>        .word   cpu_arm940_reset
> diff --git a/arch/arm/mm/proc-arm946.S b/arch/arm/mm/proc-arm946.S
> index e03f6ff..f82c7fe 100644
> --- a/arch/arm/mm/proc-arm946.S
> +++ b/arch/arm/mm/proc-arm946.S
> @@ -377,7 +377,7 @@ __arm946_setup:
>        .type   arm946_processor_functions, #object
>  ENTRY(arm946_processor_functions)
>        .word   nommu_early_abort
> -       .word   pabort_noifar
> +       .word   pabort_noifsr
>        .word   cpu_arm946_proc_init
>        .word   cpu_arm946_proc_fin
>        .word   cpu_arm946_reset
> diff --git a/arch/arm/mm/proc-arm9tdmi.S b/arch/arm/mm/proc-arm9tdmi.S
> index be6c11d..db42c52 100644
> --- a/arch/arm/mm/proc-arm9tdmi.S
> +++ b/arch/arm/mm/proc-arm9tdmi.S
> @@ -64,7 +64,7 @@ __arm9tdmi_setup:
>                .type   arm9tdmi_processor_functions, #object
>  ENTRY(arm9tdmi_processor_functions)
>                .word   nommu_early_abort
> -               .word   pabort_noifar
> +               .word   pabort_noifsr
>                .word   cpu_arm9tdmi_proc_init
>                .word   cpu_arm9tdmi_proc_fin
>                .word   cpu_arm9tdmi_reset
> --
> 1.5.4.3
>
>

[PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

It needed for proper prefetch abort handling on ARMv7.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 arch/arm/include/asm/glue.h  |    4 ++--
 arch/arm/kernel/entry-armv.S |   14 ++++----------
 arch/arm/mm/Makefile         |    2 ++
 arch/arm/mm/fault.c          |    2 +-
 arch/arm/mm/pabort-ifar.S    |   18 ++++++++++++++++++
 arch/arm/mm/pabort-noifar.S  |   18 ++++++++++++++++++
 6 files changed, 45 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm/mm/pabort-ifar.S
 create mode 100644 arch/arm/mm/pabort-noifar.S

diff --git a/arch/arm/include/asm/glue.h b/arch/arm/include/asm/glue.h
index a0e39d5..bc6595c 100644
--- a/arch/arm/include/asm/glue.h
+++ b/arch/arm/include/asm/glue.h
@@ -130,7 +130,7 @@
 # ifdef CPU_PABORT_HANDLER
 #  define MULTI_PABORT 1
 # else
-#  define CPU_PABORT_HANDLER(reg, insn)	mrc p15, 0, reg, cr6, cr0, 2
+#  define CPU_PABORT_HANDLER ifar_pabort
 # endif
 #endif
 
@@ -138,7 +138,7 @@
 # ifdef CPU_PABORT_HANDLER
 #  define MULTI_PABORT 1
 # else
-#  define CPU_PABORT_HANDLER(reg, insn)	mov reg, insn
+#  define CPU_PABORT_HANDLER noifar_pabort
 # endif
 #endif
 
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 85040cf..dbb4ce7 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -291,22 +291,16 @@ __pabt_svc:
 	tst	r3, #PSR_I_BIT
 	biceq	r9, r9, #PSR_I_BIT
 
-	@
-	@ set args, then call main handler
-	@
-	@  r0 - address of faulting instruction
-	@  r1 - pointer to registers on stack
-	@
 #ifdef MULTI_PABORT
 	mov	r0, r2			@ pass address of aborted instruction.
 	ldr	r4, .LCprocfns
 	mov	lr, pc
 	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
 #else
-	CPU_PABORT_HANDLER(r0, r2)
+	bl	CPU_PABORT_HANDLER
 #endif
 	msr	cpsr_c, r9			@ Maybe enable interrupts
-	mov	r1, sp				@ regs
+	mov	r2, sp				@ regs
 	bl	do_PrefetchAbort		@ call abort handler
 
 	@
@@ -666,10 +660,10 @@ __pabt_usr:
 	mov	lr, pc
 	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
 #else
-	CPU_PABORT_HANDLER(r0, r2)
+	bl	CPU_PABORT_HANDLER
 #endif
 	enable_irq				@ Enable interrupts
-	mov	r1, sp				@ regs
+	mov	r2, sp				@ regs
 	bl	do_PrefetchAbort		@ call abort handler
 	/* fall through */
 /*
diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index 480f78a..30f1708 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -78,3 +78,5 @@ obj-$(CONFIG_CACHE_FEROCEON_L2)	+= cache-feroceon-l2.o
 obj-$(CONFIG_CACHE_L2X0)	+= cache-l2x0.o
 obj-$(CONFIG_CACHE_XSC3L2)	+= cache-xsc3l2.o
 
+obj-$(CONFIG_CPU_PABRT_NOIFAR)	+= pabort-noifar.o
+obj-$(CONFIG_CPU_PABRT_IFAR)	+= pabort-ifar.o
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 22c9530..c7bbb32 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -480,7 +480,7 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 }
 
 asmlinkage void __exception
-do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
+do_PrefetchAbort(unsigned long addr, int ifsr, struct pt_regs *regs)
 {
 	do_translation_fault(addr, 0, regs);
 }
diff --git a/arch/arm/mm/pabort-ifar.S b/arch/arm/mm/pabort-ifar.S
new file mode 100644
index 0000000..46838ea
--- /dev/null
+++ b/arch/arm/mm/pabort-ifar.S
@@ -0,0 +1,18 @@
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+/*
+ * Function: ifar_pabort
+ *
+ * Returns : r0 = address of abort
+ *	   : r1 = IFSR
+ *
+ * Purpose : obtain information about current prefetch abort.
+ */
+	.align	5
+ENTRY(ifar_pabort)
+	mrc	p15, 0, r0, c6, c0, 2		@ get IFAR
+	mrc	p15, 0, r1, c5, c0, 1		@ get IFSR
+
+	mov	pc, lr
+ENDPROC(ifar_pabort)
diff --git a/arch/arm/mm/pabort-noifar.S b/arch/arm/mm/pabort-noifar.S
new file mode 100644
index 0000000..46b0daf
--- /dev/null
+++ b/arch/arm/mm/pabort-noifar.S
@@ -0,0 +1,18 @@
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+/*
+ * Function: noifar_pabort
+
+ * Returns : r0 = address of abort
+ *	   : r1 = 5, simulate IFSR with section translation fault status
+ *
+ * Purpose : obtain information about current prefetch abort.
+ */
+	.align	5
+ENTRY(noifar_pabort)
+	mov	r0, insn
+	mov	r1, #5		@ translation fault
+
+	mov	pc, lr
+ENDPROC(noifar_pabort)
-- 
1.6.4.4

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Aaro Koskinen]

Hello,

Kirill A. Shutemov wrote:
>>  #ifdef CONFIG_CPU_PABRT_NOIFAR
>> -# ifdef CPU_PABORT_HANDLER
>> +# ifdef CPU_PABORT_HANDLER_IFAR
>>  #  define MULTI_PABORT 1
>>  # else
>> -#  define CPU_PABORT_HANDLER(reg, insn)        mov reg, insn
>> +#  define CPU_PABORT_HANDLER_IFAR(reg, insn)   mov reg, insn
>> +#  define CPU_PABORT_HANDLER_IFSR(reg)         mrc p15, 0, reg, cr5, cr0, 1
> 
> It's incorrect. We have IFSR only on ARMv7.

It seems my assumption on the availability of that register was wrong, 
but I think it's available at least on ARMv6, and also that IFAR can be 
optional...

A.

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

On Fri, Sep 18, 2009 at 5:52 PM, Aaro Koskinen <aaro.koskinen@nokia.com> wrote:
> Hello,
>
> Kirill A. Shutemov wrote:
>>>
>>>  #ifdef CONFIG_CPU_PABRT_NOIFAR
>>> -# ifdef CPU_PABORT_HANDLER
>>> +# ifdef CPU_PABORT_HANDLER_IFAR
>>>  #  define MULTI_PABORT 1
>>>  # else
>>> -#  define CPU_PABORT_HANDLER(reg, insn)        mov reg, insn
>>> +#  define CPU_PABORT_HANDLER_IFAR(reg, insn)   mov reg, insn
>>> +#  define CPU_PABORT_HANDLER_IFSR(reg)         mrc p15, 0, reg, cr5,
>>> cr0, 1
>>
>> It's incorrect. We have IFSR only on ARMv7.
>
> It seems my assumption on the availability of that register was wrong, but I
> think it's available at least on ARMv6, and also that IFAR can be
> optional...

I can't find anything in ARMv6-M Architecture Reference Manual by
keywords "ifar" or "ifsr".

>
> A.
>

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Catalin Marinas]

On Fri, 2009-09-18 at 18:08 +0300, Kirill A. Shutemov wrote:
> On Fri, Sep 18, 2009 at 5:52 PM, Aaro Koskinen <aaro.koskinen@nokia.com> wrote:
> > Hello,
> >
> > Kirill A. Shutemov wrote:
> >>>
> >>>  #ifdef CONFIG_CPU_PABRT_NOIFAR
> >>> -# ifdef CPU_PABORT_HANDLER
> >>> +# ifdef CPU_PABORT_HANDLER_IFAR
> >>>  #  define MULTI_PABORT 1
> >>>  # else
> >>> -#  define CPU_PABORT_HANDLER(reg, insn)        mov reg, insn
> >>> +#  define CPU_PABORT_HANDLER_IFAR(reg, insn)   mov reg, insn
> >>> +#  define CPU_PABORT_HANDLER_IFSR(reg)         mrc p15, 0, reg, cr5,
> >>> cr0, 1
> >>
> >> It's incorrect. We have IFSR only on ARMv7.
> >
> > It seems my assumption on the availability of that register was wrong, but I
> > think it's available at least on ARMv6, and also that IFAR can be
> > optional...
> 
> I can't find anything in ARMv6-M Architecture Reference Manual by
> keywords "ifar" or "ifsr".

ARMv6-M is for the M-profile CPUs (Thumb or Thumb-2 only ISA, no MMU ,
it doesn't even have CP15).

You would need to check the A profile. Try the ARMv7-AR reference manual
(now freely available, though it needs a click-through) which has a
section on differences with the ARMv6 as well.

-- 
Catalin

[PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

It needed for proper prefetch abort handling on ARMv7.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 arch/arm/include/asm/glue.h    |   10 ++++++++--
 arch/arm/kernel/entry-armv.S   |   14 ++++----------
 arch/arm/kernel/entry-common.S |    8 ++++++--
 arch/arm/mm/fault.c            |    2 +-
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/glue.h b/arch/arm/include/asm/glue.h
index a0e39d5..5bfaf6f 100644
--- a/arch/arm/include/asm/glue.h
+++ b/arch/arm/include/asm/glue.h
@@ -130,7 +130,10 @@
 # ifdef CPU_PABORT_HANDLER
 #  define MULTI_PABORT 1
 # else
-#  define CPU_PABORT_HANDLER(reg, insn)	mrc p15, 0, reg, cr6, cr0, 2
+#  define CPU_PABORT_HANDLER(addr, ifsr, insn) \
+	mrc	p15, 0, addr, c6, c0, 2 ; \
+	mrc	p15, 0, ifsr, c5, c0, 1
+
 # endif
 #endif
 
@@ -138,7 +141,10 @@
 # ifdef CPU_PABORT_HANDLER
 #  define MULTI_PABORT 1
 # else
-#  define CPU_PABORT_HANDLER(reg, insn)	mov reg, insn
+#define __hash_5 #5
+#  define CPU_PABORT_HANDLER(addr, ifsr, insn) \
+	mov	addr, insn ; \
+	mov	ifsr, __hash_5
 # endif
 #endif
 
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 3d727a8..335ae58 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -303,22 +303,16 @@ __pabt_svc:
 	tst	r3, #PSR_I_BIT
 	biceq	r9, r9, #PSR_I_BIT
 
-	@
-	@ set args, then call main handler
-	@
-	@  r0 - address of faulting instruction
-	@  r1 - pointer to registers on stack
-	@
 #ifdef MULTI_PABORT
 	mov	r0, r2			@ pass address of aborted instruction.
 	ldr	r4, .LCprocfns
 	mov	lr, pc
 	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
 #else
-	CPU_PABORT_HANDLER(r0, r2)
+	CPU_PABORT_HANDLER(r0, r1, r2)
 #endif
 	msr	cpsr_c, r9			@ Maybe enable interrupts
-	mov	r1, sp				@ regs
+	mov	r2, sp				@ regs
 	bl	do_PrefetchAbort		@ call abort handler
 
 	@
@@ -697,10 +691,10 @@ __pabt_usr:
 	mov	lr, pc
 	ldr	pc, [r4, #PROCESSOR_PABT_FUNC]
 #else
-	CPU_PABORT_HANDLER(r0, r2)
+	CPU_PABORT_HANDLER(r0, r1, r2)
 #endif
 	enable_irq				@ Enable interrupts
-	mov	r1, sp				@ regs
+	mov	r2, sp				@ regs
 	bl	do_PrefetchAbort		@ call abort handler
  UNWIND(.fnend		)
 	/* fall through */
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 807cfeb..931ab9a 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -426,10 +426,14 @@ sys_mmap2:
 ENDPROC(sys_mmap2)
 
 ENTRY(pabort_ifar)
-		mrc	p15, 0, r0, cr6, cr0, 2
-ENTRY(pabort_noifar)
+		mrc	p15, 0, r0, c6, c0, 2		@ get IFAR
+		mrc	p15, 0, r1, c5, c0, 1		@ get IFSR
 		mov	pc, lr
 ENDPROC(pabort_ifar)
+ENTRY(pabort_noifar)
+		/* simulate IFSR with section translation fault status */
+		mov	r1, #5
+		mov	pc, lr
 ENDPROC(pabort_noifar)
 
 #ifdef CONFIG_OABI_COMPAT
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index cc8829d..5e27462 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -506,7 +506,7 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 }
 
 asmlinkage void __exception
-do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
+do_PrefetchAbort(unsigned long addr, int ifsr, struct pt_regs *regs)
 {
 	do_translation_fault(addr, 0, regs);
 }
-- 
1.6.4.4

[PATCH 2/2] ARM: Proper prefetch abort handling on ARMv7

[Kirill A. Shutemov]

Currently, on ARMv7, if an application tries to execute code
(or garbage) on non-executable page it hangs. It caused by incorrect
prefetch abort handling. Now every prefetch abort processes as
a translation fault.

To fix this we have to analize instruction fault status register
to figure out reason why we've got the abort and process it
accordingly.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 arch/arm/mm/fault.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 5e27462..5abb9a5 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -505,9 +505,58 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	arm_notify_die("", regs, &info, fsr, 0);
 }
 
+
+static struct fsr_info ifsr_info[] = {
+	{ do_bad,		SIGBUS,  0,		"unknown 0"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 1"			   },
+	{ do_bad,		SIGBUS,  0,		"debug event"			   },
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"section access flag fault"	   },
+	{ do_bad,		SIGBUS,  0,		"unknown 4"			   },
+	{ do_translation_fault,	SIGSEGV, SEGV_MAPERR,	"section translation fault"	   },
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"page access flag fault"	   },
+	{ do_page_fault,	SIGSEGV, SEGV_MAPERR,	"page translation fault"	   },
+	{ do_bad,		SIGBUS,	 0,		"external abort on non-linefetch"  },
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"section domain fault"		   },
+	{ do_bad,		SIGBUS,  0,		"unknown 10"			   },
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"page domain fault"		   },
+	{ do_bad,		SIGBUS,	 0,		"external abort on translation"	   },
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"section permission fault"	   },
+	{ do_bad,		SIGBUS,	 0,		"external abort on translation"	   },
+	{ do_bad,		SIGSEGV, SEGV_ACCERR,	"page permission fault"		   },
+	{ do_bad,		SIGBUS,  0,		"unknown 16"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 17"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 18"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 19"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 20"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 21"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 22"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 23"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 24"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 25"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 26"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 27"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 28"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 29"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 30"			   },
+	{ do_bad,		SIGBUS,  0,		"unknown 31"			   },
+};
+
 asmlinkage void __exception
 do_PrefetchAbort(unsigned long addr, int ifsr, struct pt_regs *regs)
 {
-	do_translation_fault(addr, 0, regs);
+	const struct fsr_info *inf = ifsr_info + (ifsr & 15) + ((ifsr & (1 << 10)) >> 6);
+	struct siginfo info;
+
+	if (!inf->fn(addr, ifsr, regs))
+		return;
+
+	printk(KERN_ALERT "Unhandled prefetch abort: %s (0x%03x) at 0x%08lx\n",
+		inf->name, ifsr, addr);
+
+	info.si_signo = inf->sig;
+	info.si_errno = 0;
+	info.si_code  = inf->code;
+	info.si_addr  = (void __user *)addr;
+	arm_notify_die("", regs, &info, ifsr, 0);
 }
 
-- 
1.6.4.4

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Russell King - ARM Linux]

On Fri, Sep 18, 2009 at 11:55:42PM +0300, Kirill A. Shutemov wrote:
> It needed for proper prefetch abort handling on ARMv7.

I think the only thing which is missing is an explaination about why
this is desirable given that only later CPUs can give this additional
information.

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Russell King - ARM Linux]

On Sat, Sep 19, 2009 at 12:03:16PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 18, 2009 at 11:55:42PM +0300, Kirill A. Shutemov wrote:
> > It needed for proper prefetch abort handling on ARMv7.
> 
> I think the only thing which is missing is an explaination about why
> this is desirable given that only later CPUs can give this additional
> information.

So you've posted it to the patch system, without further discussion here.

I think the solution is wrong - it makes instruction permission faults
unnecessarily noisy, which is not what the decoding table is supposed
to be doing.  The decoding table's bad entries are there to catch those
_unexpected_ cases.

Instead, I suggest that you have a look at this:

        if (fsr & (1 << 11)) /* write? */
                mask = VM_WRITE;
        else
                mask = VM_READ|VM_EXEC|VM_WRITE;

        fault = VM_FAULT_BADACCESS;
        if (!(vma->vm_flags & mask))
                goto out;

in __do_page_fault - if we are handling a prefetch abort, we really only
want to check that the VMA has VM_EXEC permission, not that it can be
read and written as well.

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

On Sun, Sep 20, 2009 at 11:15 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Sep 19, 2009 at 12:03:16PM +0100, Russell King - ARM Linux wrote:
>> On Fri, Sep 18, 2009 at 11:55:42PM +0300, Kirill A. Shutemov wrote:
>> > It needed for proper prefetch abort handling on ARMv7.
>>
>> I think the only thing which is missing is an explaination about why
>> this is desirable given that only later CPUs can give this additional
>> information.
>
> So you've posted it to the patch system, without further discussion here.
>
> I think the solution is wrong - it makes instruction permission faults
> unnecessarily noisy, which is not what the decoding table is supposed
> to be doing.  The decoding table's bad entries are there to catch those
> _unexpected_ cases.
>
> Instead, I suggest that you have a look at this:
>
>        if (fsr & (1 << 11)) /* write? */
>                mask = VM_WRITE;
>        else
>                mask = VM_READ|VM_EXEC|VM_WRITE;
>
>        fault = VM_FAULT_BADACCESS;
>        if (!(vma->vm_flags & mask))
>                goto out;
>
> in __do_page_fault - if we are handling a prefetch abort, we really only
> want to check that the VMA has VM_EXEC permission, not that it can be
> read and written as well.
>

Ok, so __do_page_fault() should know where we are: in data abort or in
prefetch abort. What is right way to do it? Should we create one more
argument or use one of reserved bits IFSR?

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Russell King - ARM Linux]

On Sun, Sep 20, 2009 at 12:35:19PM +0300, Kirill A. Shutemov wrote:
> Ok, so __do_page_fault() should know where we are: in data abort or in
> prefetch abort. What is right way to do it? Should we create one more
> argument or use one of reserved bits IFSR?

Well, this is my solution to the problem - could you test it please?
This patch is actually the result of several patches merged together,
so I won't supply a proper description etc.

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index cc8829d..8fbb22d 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -25,6 +25,19 @@
 
 #include "fault.h"
 
+/*
+ * Fault status register encodings.  We steal bit 31 for our own purposes.
+ */
+#define FSR_LNX_PF		(1 << 31)
+#define FSR_WRITE		(1 << 11)
+#define FSR_FS4			(1 << 10)
+#define FSR_FS3_0		(15)
+
+static inline int fsr_fs(unsigned int fsr)
+{
+	return (fsr & FSR_FS3_0) | (fsr & FSR_FS4) >> 6;
+}
+
 #ifdef CONFIG_MMU
 
 #ifdef CONFIG_KPROBES
@@ -182,18 +195,35 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 #define VM_FAULT_BADMAP		0x010000
 #define VM_FAULT_BADACCESS	0x020000
 
-static int
+/*
+ * Check that the permissions on the VMA allow for the fault which occurred.
+ * If we encountered a write fault, we must have write permission, otherwise
+ * we allow any permission.
+ */
+static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
+{
+	unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+
+	if (fsr & FSR_WRITE)
+		mask = VM_WRITE;
+	if (fsr & FSR_LNX_PF)
+		mask = VM_EXEC;
+
+	return vma->vm_flags & mask ? false : true;
+}
+
+static noinline int __kprobes
 __do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
 		struct task_struct *tsk)
 {
 	struct vm_area_struct *vma;
-	int fault, mask;
+	int fault;
 
 	vma = find_vma(mm, addr);
 	fault = VM_FAULT_BADMAP;
-	if (!vma)
+	if (unlikely(!vma))
 		goto out;
-	if (vma->vm_start > addr)
+	if (unlikely(vma->vm_start > addr))
 		goto check_stack;
 
 	/*
@@ -201,47 +231,24 @@ __do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
 	 * memory access, so we can handle it.
 	 */
 good_area:
-	if (fsr & (1 << 11)) /* write? */
-		mask = VM_WRITE;
-	else
-		mask = VM_READ|VM_EXEC|VM_WRITE;
-
-	fault = VM_FAULT_BADACCESS;
-	if (!(vma->vm_flags & mask))
+	if (access_error(fsr, vma)) {
+		fault = VM_FAULT_BADACCESS;
 		goto out;
+	}
 
 	/*
-	 * If for any reason at all we couldn't handle
-	 * the fault, make sure we exit gracefully rather
-	 * than endlessly redo the fault.
+	 * If for any reason at all we couldn't handle the fault, make
+	 * sure we exit gracefully rather than endlessly redo the fault.
 	 */
-survive:
-	fault = handle_mm_fault(mm, vma, addr & PAGE_MASK, (fsr & (1 << 11)) ? FAULT_FLAG_WRITE : 0);
-	if (unlikely(fault & VM_FAULT_ERROR)) {
-		if (fault & VM_FAULT_OOM)
-			goto out_of_memory;
-		else if (fault & VM_FAULT_SIGBUS)
-			return fault;
-		BUG();
-	}
+	fault = handle_mm_fault(mm, vma, addr & PAGE_MASK, (fsr & FSR_WRITE) ? FAULT_FLAG_WRITE : 0);
+	if (unlikely(fault & VM_FAULT_ERROR))
+		return fault;
 	if (fault & VM_FAULT_MAJOR)
 		tsk->maj_flt++;
 	else
 		tsk->min_flt++;
 	return fault;
 
-out_of_memory:
-	if (!is_global_init(tsk))
-		goto out;
-
-	/*
-	 * If we are out of memory for pid1, sleep for a while and retry
-	 */
-	up_read(&mm->mmap_sem);
-	yield();
-	down_read(&mm->mmap_sem);
-	goto survive;
-
 check_stack:
 	if (vma->vm_flags & VM_GROWSDOWN && !expand_stack(vma, addr))
 		goto good_area;
@@ -278,6 +285,13 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		if (!user_mode(regs) && !search_exception_tables(regs->ARM_pc))
 			goto no_context;
 		down_read(&mm->mmap_sem);
+	} else {
+		/*
+		 * The above down_read_trylock() might have succeeded in
+		 * which case, we'll have missed the might_sleep() from
+		 * down_read()
+		 */
+		might_sleep();
 	}
 
 	fault = __do_page_fault(mm, addr, fsr, tsk);
@@ -289,6 +303,16 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (likely(!(fault & (VM_FAULT_ERROR | VM_FAULT_BADMAP | VM_FAULT_BADACCESS))))
 		return 0;
 
+	if (fault & VM_FAULT_OOM) {
+		/*
+		 * We ran out of memory, call the OOM killer, and return to
+		 * userspace (which will retry the fault, or kill us if we
+		 * got oom-killed)
+		 */
+		pagefault_out_of_memory();
+		return 0;
+	}
+
 	/*
 	 * If we are in kernel mode at this point, we
 	 * have no context to handle this fault with.
@@ -296,16 +320,6 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (!user_mode(regs))
 		goto no_context;
 
-	if (fault & VM_FAULT_OOM) {
-		/*
-		 * We ran out of memory, or some other thing
-		 * happened to us that made us unable to handle
-		 * the page fault gracefully.
-		 */
-		printk("VM: killing process %s\n", tsk->comm);
-		do_group_exit(SIGKILL);
-		return 0;
-	}
 	if (fault & VM_FAULT_SIGBUS) {
 		/*
 		 * We had some memory, but were unable to
@@ -489,10 +503,10 @@ hook_fault_code(int nr, int (*fn)(unsigned long, unsigned int, struct pt_regs *)
 asmlinkage void __exception
 do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
-	const struct fsr_info *inf = fsr_info + (fsr & 15) + ((fsr & (1 << 10)) >> 6);
+	const struct fsr_info *inf = fsr_info + fsr_fs(fsr);
 	struct siginfo info;
 
-	if (!inf->fn(addr, fsr, regs))
+	if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))
 		return;
 
 	printk(KERN_ALERT "Unhandled fault: %s (0x%03x) at 0x%08lx\n",
@@ -508,6 +522,6 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 asmlinkage void __exception
 do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
 {
-	do_translation_fault(addr, 0, regs);
+	do_translation_fault(addr, FSR_LNX_PF, regs);
 }
 

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

On Sun, Sep 20, 2009 at 3:24 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Sep 20, 2009 at 12:35:19PM +0300, Kirill A. Shutemov wrote:
>> Ok, so __do_page_fault() should know where we are: in data abort or in
>> prefetch abort. What is right way to do it? Should we create one more
>> argument or use one of reserved bits IFSR?
>
> Well, this is my solution to the problem - could you test it please?

I'll test it on Monday.

Do you want to ignore IFSR? I don't think that it's a good idea. We will
get infinite loop of faults on an unexpected for kernel type of fault, like
we have now for permission faults. Better to call do_bad() in this case.

> This patch is actually the result of several patches merged together,
> so I won't supply a proper description etc.
>
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index cc8829d..8fbb22d 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -25,6 +25,19 @@
>
>  #include "fault.h"
>
> +/*
> + * Fault status register encodings.  We steal bit 31 for our own purposes.
> + */
> +#define FSR_LNX_PF             (1 << 31)
> +#define FSR_WRITE              (1 << 11)
> +#define FSR_FS4                        (1 << 10)
> +#define FSR_FS3_0              (15)
> +
> +static inline int fsr_fs(unsigned int fsr)
> +{
> +       return (fsr & FSR_FS3_0) | (fsr & FSR_FS4) >> 6;
> +}
> +
>  #ifdef CONFIG_MMU
>
>  #ifdef CONFIG_KPROBES
> @@ -182,18 +195,35 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  #define VM_FAULT_BADMAP                0x010000
>  #define VM_FAULT_BADACCESS     0x020000
>
> -static int
> +/*
> + * Check that the permissions on the VMA allow for the fault which occurred.
> + * If we encountered a write fault, we must have write permission, otherwise
> + * we allow any permission.
> + */
> +static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
> +{
> +       unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
> +
> +       if (fsr & FSR_WRITE)
> +               mask = VM_WRITE;
> +       if (fsr & FSR_LNX_PF)
> +               mask = VM_EXEC;
> +
> +       return vma->vm_flags & mask ? false : true;
> +}
> +
> +static noinline int __kprobes
>  __do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
>                struct task_struct *tsk)
>  {
>        struct vm_area_struct *vma;
> -       int fault, mask;
> +       int fault;
>
>        vma = find_vma(mm, addr);
>        fault = VM_FAULT_BADMAP;
> -       if (!vma)
> +       if (unlikely(!vma))
>                goto out;
> -       if (vma->vm_start > addr)
> +       if (unlikely(vma->vm_start > addr))
>                goto check_stack;
>
>        /*
> @@ -201,47 +231,24 @@ __do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
>         * memory access, so we can handle it.
>         */
>  good_area:
> -       if (fsr & (1 << 11)) /* write? */
> -               mask = VM_WRITE;
> -       else
> -               mask = VM_READ|VM_EXEC|VM_WRITE;
> -
> -       fault = VM_FAULT_BADACCESS;
> -       if (!(vma->vm_flags & mask))
> +       if (access_error(fsr, vma)) {
> +               fault = VM_FAULT_BADACCESS;
>                goto out;
> +       }
>
>        /*
> -        * If for any reason at all we couldn't handle
> -        * the fault, make sure we exit gracefully rather
> -        * than endlessly redo the fault.
> +        * If for any reason at all we couldn't handle the fault, make
> +        * sure we exit gracefully rather than endlessly redo the fault.
>         */
> -survive:
> -       fault = handle_mm_fault(mm, vma, addr & PAGE_MASK, (fsr & (1 << 11)) ? FAULT_FLAG_WRITE : 0);
> -       if (unlikely(fault & VM_FAULT_ERROR)) {
> -               if (fault & VM_FAULT_OOM)
> -                       goto out_of_memory;
> -               else if (fault & VM_FAULT_SIGBUS)
> -                       return fault;
> -               BUG();
> -       }
> +       fault = handle_mm_fault(mm, vma, addr & PAGE_MASK, (fsr & FSR_WRITE) ? FAULT_FLAG_WRITE : 0);
> +       if (unlikely(fault & VM_FAULT_ERROR))
> +               return fault;
>        if (fault & VM_FAULT_MAJOR)
>                tsk->maj_flt++;
>        else
>                tsk->min_flt++;
>        return fault;
>
> -out_of_memory:
> -       if (!is_global_init(tsk))
> -               goto out;
> -
> -       /*
> -        * If we are out of memory for pid1, sleep for a while and retry
> -        */
> -       up_read(&mm->mmap_sem);
> -       yield();
> -       down_read(&mm->mmap_sem);
> -       goto survive;
> -
>  check_stack:
>        if (vma->vm_flags & VM_GROWSDOWN && !expand_stack(vma, addr))
>                goto good_area;
> @@ -278,6 +285,13 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>                if (!user_mode(regs) && !search_exception_tables(regs->ARM_pc))
>                        goto no_context;
>                down_read(&mm->mmap_sem);
> +       } else {
> +               /*
> +                * The above down_read_trylock() might have succeeded in
> +                * which case, we'll have missed the might_sleep() from
> +                * down_read()
> +                */
> +               might_sleep();
>        }
>
>        fault = __do_page_fault(mm, addr, fsr, tsk);
> @@ -289,6 +303,16 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>        if (likely(!(fault & (VM_FAULT_ERROR | VM_FAULT_BADMAP | VM_FAULT_BADACCESS))))
>                return 0;
>
> +       if (fault & VM_FAULT_OOM) {
> +               /*
> +                * We ran out of memory, call the OOM killer, and return to
> +                * userspace (which will retry the fault, or kill us if we
> +                * got oom-killed)
> +                */
> +               pagefault_out_of_memory();
> +               return 0;
> +       }
> +
>        /*
>         * If we are in kernel mode at this point, we
>         * have no context to handle this fault with.
> @@ -296,16 +320,6 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>        if (!user_mode(regs))
>                goto no_context;
>
> -       if (fault & VM_FAULT_OOM) {
> -               /*
> -                * We ran out of memory, or some other thing
> -                * happened to us that made us unable to handle
> -                * the page fault gracefully.
> -                */
> -               printk("VM: killing process %s\n", tsk->comm);
> -               do_group_exit(SIGKILL);
> -               return 0;
> -       }
>        if (fault & VM_FAULT_SIGBUS) {
>                /*
>                 * We had some memory, but were unable to
> @@ -489,10 +503,10 @@ hook_fault_code(int nr, int (*fn)(unsigned long, unsigned int, struct pt_regs *)
>  asmlinkage void __exception
>  do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  {
> -       const struct fsr_info *inf = fsr_info + (fsr & 15) + ((fsr & (1 << 10)) >> 6);
> +       const struct fsr_info *inf = fsr_info + fsr_fs(fsr);
>        struct siginfo info;
>
> -       if (!inf->fn(addr, fsr, regs))
> +       if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))
>                return;
>
>        printk(KERN_ALERT "Unhandled fault: %s (0x%03x) at 0x%08lx\n",
> @@ -508,6 +522,6 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  asmlinkage void __exception
>  do_PrefetchAbort(unsigned long addr, struct pt_regs *regs)
>  {
> -       do_translation_fault(addr, 0, regs);
> +       do_translation_fault(addr, FSR_LNX_PF, regs);
>  }
>
>
>

Re: [PATCH 1/2] ARM: Pass IFSR register to do_PrefetchAbort()

[Kirill A. Shutemov]

On Sun, Sep 20, 2009 at 3:24 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Sep 20, 2009 at 12:35:19PM +0300, Kirill A. Shutemov wrote:
>> Ok, so __do_page_fault() should know where we are: in data abort or in
>> prefetch abort. What is right way to do it? Should we create one more
>> argument or use one of reserved bits IFSR?
>
> Well, this is my solution to the problem - could you test it please?

Yes, it works. But I still think that ignoring IFSR is a mistake. I'll send
updated patches