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@xxxxxxxxx> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/