2009-09-18 10:48:08

by Kirill A. Shutemov

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

It needed for proper prefetch abort handling on ARMv7.

Signed-off-by: Kirill A. Shutemov <[email protected]>
---
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


2009-09-18 11:19:29

by Aaro Koskinen

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

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.

2009-09-18 11:28:55

by Aaro Koskinen

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

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

2009-09-18 11:36:35

by Kirill A. Shutemov

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

On Fri, Sep 18, 2009 at 2:18 PM, Aaro Koskinen <[email protected]> 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.

2009-09-18 11:45:28

by Kirill A. Shutemov

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

On Fri, Sep 18, 2009 at 2:28 PM, Aaro Koskinen <[email protected]> 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
>
>

2009-09-18 10:48:18

by Kirill A. Shutemov

[permalink] [raw]
Subject: [PATCH 2/2] ARM: Proper prefetch abort handling

Handle prefetch abort basing on instruction fault status register.

Now we can process permission fault correctly.

Signed-off-by: Kirill A. Shutemov <[email protected]>
---
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 c7bbb32..b2574cf 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -479,9 +479,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 fault: %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

2009-09-18 14:53:10

by Aaro Koskinen

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

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.

2009-09-18 15:08:52

by Kirill A. Shutemov

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

On Fri, Sep 18, 2009 at 5:52 PM, Aaro Koskinen <[email protected]> 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.
>

2009-09-18 15:38:32

by Catalin Marinas

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

On Fri, 2009-09-18 at 18:08 +0300, Kirill A. Shutemov wrote:
> On Fri, Sep 18, 2009 at 5:52 PM, Aaro Koskinen <[email protected]> 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