ARM errata 430973 on multi platform kernels

Commit Message

Russell King - ARM Linux April 9, 2015, 1:48 p.m. UTC

On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > Works for me. The above needs the following fix folded in to build:
> > 
> > --- a/arch/arm/mm/proc-v7.S
> > +++ b/arch/arm/mm/proc-v7.S
> > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> >  __v7_ca8_proc_info:
> >  	.long	0x410fc080
> >  	.long	0xff0ffff0
> > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> >  
> >  #endif	/* CONFIG_ARM_LPAE */
> 
> Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

8<===
From: Russell King <[e-mail]>
Subject: [PATCH] ARM: proc-v7: avoid errata 430973 workaround for non-Cortex
 A8 CPUs

Avoid the errata 430973 workaround for non-Cortex A8 CPUs.  Having this
workaround enabled introduces an additional branch target buffer flush
into the context switching path, something we wish to avoid.  To allow
this errata to be enabled in multiplatform kernels while reducing its
impact, rearrange the Cortex-A8 CPU support to avoid impacting on other
Version 7 CPUs.

Signed-off-by: Russell King <[e-mail]>
---
 arch/arm/mm/proc-v7-2level.S | 12 ++++++++----
 arch/arm/mm/proc-v7.S        | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

Comments

Tony Lindgren April 9, 2015, 3:09 p.m. UTC | #1

* Russell King - ARM Linux <[e-mail]> [150409 06:49]:
> On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > Works for me. The above needs the following fix folded in to build:
> > > 
> > > --- a/arch/arm/mm/proc-v7.S
> > > +++ b/arch/arm/mm/proc-v7.S
> > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > >  __v7_ca8_proc_info:
> > >  	.long	0x410fc080
> > >  	.long	0xff0ffff0
> > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > >  
> > >  #endif	/* CONFIG_ARM_LPAE */
> > 
> > Thanks, merged into the original patch.
> 
> Do you want to give me an ack for this, thanks?

The patch below works for me:

Tested-by: Tony Lindgren <[e-mail]>

I'm wondering if this and the follow-up patch should be tagged
cc: stable?

They together fix apps segfaulting both with and without 430973
for some common use cases for distro kernels.

Regards,

Tony
 
> 8<===
> From: Russell King <[e-mail]>
> Subject: [PATCH] ARM: proc-v7: avoid errata 430973 workaround for non-Cortex
>  A8 CPUs
> 
> Avoid the errata 430973 workaround for non-Cortex A8 CPUs.  Having this
> workaround enabled introduces an additional branch target buffer flush
> into the context switching path, something we wish to avoid.  To allow
> this errata to be enabled in multiplatform kernels while reducing its
> impact, rearrange the Cortex-A8 CPU support to avoid impacting on other
> Version 7 CPUs.
> 
> Signed-off-by: Russell King <[e-mail]>
> ---
>  arch/arm/mm/proc-v7-2level.S | 12 ++++++++----
>  arch/arm/mm/proc-v7.S        | 28 ++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
> index bc86be205c04..fa385140715f 100644
> --- a/arch/arm/mm/proc-v7-2level.S
> +++ b/arch/arm/mm/proc-v7-2level.S
> @@ -37,15 +37,18 @@
>   *	It is assumed that:
>   *	- we are not using split page tables
>   */
> -ENTRY(cpu_v7_switch_mm)
> +ENTRY(cpu_ca8_switch_mm)
>  #ifdef CONFIG_MMU
>  	mov	r2, #0
> -	mmid	r1, r1				@ get mm->context.id
> -	ALT_SMP(orr	r0, r0, #TTB_FLAGS_SMP)
> -	ALT_UP(orr	r0, r0, #TTB_FLAGS_UP)
>  #ifdef CONFIG_ARM_ERRATA_430973
>  	mcr	p15, 0, r2, c7, c5, 6		@ flush BTAC/BTB
>  #endif
> +#endif
> +ENTRY(cpu_v7_switch_mm)
> +#ifdef CONFIG_MMU
> +	mmid	r1, r1				@ get mm->context.id
> +	ALT_SMP(orr	r0, r0, #TTB_FLAGS_SMP)
> +	ALT_UP(orr	r0, r0, #TTB_FLAGS_UP)
>  #ifdef CONFIG_PID_IN_CONTEXTIDR
>  	mrc	p15, 0, r2, c13, c0, 1		@ read current context ID
>  	lsr	r2, r2, #8			@ extract the PID
> @@ -61,6 +64,7 @@ ENTRY(cpu_v7_switch_mm)
>  #endif
>  	bx	lr
>  ENDPROC(cpu_v7_switch_mm)
> +ENDPROC(cpu_ca8_switch_mm)
>  
>  /*
>   *	cpu_v7_set_pte_ext(ptep, pte)
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index b1d19ea5e1af..003190ae9cd8 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -153,6 +153,21 @@ ENDPROC(cpu_v7_do_resume)
>  #endif
>  
>  /*
> + * Cortex-A8
> + */
> +	globl_equ	cpu_ca8_proc_init,	cpu_v7_proc_init
> +	globl_equ	cpu_ca8_proc_fin,	cpu_v7_proc_fin
> +	globl_equ	cpu_ca8_reset,		cpu_v7_reset
> +	globl_equ	cpu_ca8_do_idle,	cpu_v7_do_idle
> +	globl_equ	cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
> +	globl_equ	cpu_ca8_set_pte_ext,	cpu_v7_set_pte_ext
> +	globl_equ	cpu_ca8_suspend_size,	cpu_v7_suspend_size
> +#ifdef CONFIG_ARM_CPU_SUSPEND
> +	globl_equ	cpu_ca8_do_suspend,	cpu_v7_do_suspend
> +	globl_equ	cpu_ca8_do_resume,	cpu_v7_do_resume
> +#endif
> +
> +/*
>   * Cortex-A9 processor functions
>   */
>  	globl_equ	cpu_ca9mp_proc_init,	cpu_v7_proc_init
> @@ -471,7 +486,10 @@ __v7_setup_stack:
>  
>  	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
>  	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> +#ifndef CONFIG_ARM_LPAE
> +	define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
>  	define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> +#endif
>  #ifdef CONFIG_CPU_PJ4B
>  	define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
>  #endif
> @@ -527,6 +545,16 @@ __v7_ca9mp_proc_info:
>  	__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
>  	.size	__v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info
>  
> +	/*
> +	 * ARM Ltd. Cortex A8 processor.
> +	 */
> +	.type	__v7_ca8_proc_info, #object
> +__v7_ca8_proc_info:
> +	.long	0x410fc080
> +	.long	0xff0ffff0
> +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> +	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> +
>  #endif	/* CONFIG_ARM_LPAE */
>  
>  	/*
> -- 
> 1.8.3.1
> 
> -- 
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Russell King - ARM Linux April 9, 2015, 3:30 p.m. UTC | #2

On Thu, Apr 09, 2015 at 08:09:19AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <[e-mail]> [150409 06:49]:
> > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > Works for me. The above needs the following fix folded in to build:
> > > > 
> > > > --- a/arch/arm/mm/proc-v7.S
> > > > +++ b/arch/arm/mm/proc-v7.S
> > > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > > >  __v7_ca8_proc_info:
> > > >  	.long	0x410fc080
> > > >  	.long	0xff0ffff0
> > > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > > >  
> > > >  #endif	/* CONFIG_ARM_LPAE */
> > > 
> > > Thanks, merged into the original patch.
> > 
> > Do you want to give me an ack for this, thanks?
> 
> The patch below works for me:
> 
> Tested-by: Tony Lindgren <[e-mail]>
> 
> I'm wondering if this and the follow-up patch should be tagged
> cc: stable?
> 
> They together fix apps segfaulting both with and without 430973
> for some common use cases for distro kernels.

If we do, I'll want to merge both patches together...  For the time being,
I'll queue it without the stable tag.

Sebastian Reichel April 15, 2015, 4:31 p.m. UTC | #3

Hi,

On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > Works for me. The above needs the following fix folded in to build:
> > > 
> > > --- a/arch/arm/mm/proc-v7.S
> > > +++ b/arch/arm/mm/proc-v7.S
> > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > >  __v7_ca8_proc_info:
> > >  	.long	0x410fc080
> > >  	.long	0xff0ffff0
> > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > >  
> > >  #endif	/* CONFIG_ARM_LPAE */
> > 
> > Thanks, merged into the original patch.
> 
> Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
this after applying the patch to v4.0:

sre@earth ~/src/linux [430973-fix] % make -j4
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  AS      arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S: Assembler messages:
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

-- Sebastian

Tony Lindgren April 16, 2015, 4:08 p.m. UTC | #4

* Sebastian Reichel <[e-mail]> [150415 09:32]:
> Hi,
> 
> On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > Works for me. The above needs the following fix folded in to build:
> > > > 
> > > > --- a/arch/arm/mm/proc-v7.S
> > > > +++ b/arch/arm/mm/proc-v7.S
> > > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > > >  __v7_ca8_proc_info:
> > > >  	.long	0x410fc080
> > > >  	.long	0xff0ffff0
> > > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > > >  
> > > >  #endif	/* CONFIG_ARM_LPAE */
> > > 
> > > Thanks, merged into the original patch.
> > 
> > Do you want to give me an ack for this, thanks?
> 
> I tried to test this together with Tony's follow up patch, but I get
> this after applying the patch to v4.0:
> 
> sre@earth ~/src/linux [430973-fix] % make -j4
>   CHK     include/config/kernel.release
>   CHK     include/generated/uapi/linux/version.h
>   CHK     include/generated/utsrelease.h
> make[1]: 'include/generated/mach-types.h' is up to date.
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   AS      arch/arm/mm/proc-v7.o
> arch/arm/mm/proc-v7.S: Assembler messages:
> arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
> make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
> Makefile:947: recipe for target 'arch/arm/mm' failed
> make: *** [arch/arm/mm] Error 2
> make: *** Waiting for unfinished jobs....

Maybe test the version in Linux next:

a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

And then apply my patch on top.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sebastian Reichel April 17, 2015, 6:41 p.m. UTC | #5

On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <[e-mail]> [150415 09:32]:
> > Hi,
> > 
> > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > Works for me. The above needs the following fix folded in to build:
> > > > > 
> > > > > --- a/arch/arm/mm/proc-v7.S
> > > > > +++ b/arch/arm/mm/proc-v7.S
> > > > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > > > >  __v7_ca8_proc_info:
> > > > >  	.long	0x410fc080
> > > > >  	.long	0xff0ffff0
> > > > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > > > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > > > >  
> > > > >  #endif	/* CONFIG_ARM_LPAE */
> > > > 
> > > > Thanks, merged into the original patch.
> > > 
> > > Do you want to give me an ack for this, thanks?
> > 
> > I tried to test this together with Tony's follow up patch, but I get
> > this after applying the patch to v4.0:
> > 
> > sre@earth ~/src/linux [430973-fix] % make -j4
> >   CHK     include/config/kernel.release
> >   CHK     include/generated/uapi/linux/version.h
> >   CHK     include/generated/utsrelease.h
> > make[1]: 'include/generated/mach-types.h' is up to date.
> >   CALL    scripts/checksyscalls.sh
> >   CHK     include/generated/compile.h
> >   AS      arch/arm/mm/proc-v7.o
> > arch/arm/mm/proc-v7.S: Assembler messages:
> > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
> > make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
> > Makefile:947: recipe for target 'arch/arm/mm' failed
> > make: *** [arch/arm/mm] Error 2
> > make: *** Waiting for unfinished jobs....
> 
> Maybe test the version in Linux next:
> 
> a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

DONE with your your patch added on top:

Tested-By: Sebastian Reichel <[e-mail]>

(on N900)

I guess we should also drop the CONFIG_ARM_ERRATA_430973 check from
pdata-quirks' nokia_n900_legacy_init() and just enable it unconditionally.

-- Sebastian

Tony Lindgren April 20, 2015, 11:40 p.m. UTC | #6

* Sebastian Reichel <[e-mail]> [150417 11:43]:
> On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <[e-mail]> [150415 09:32]:
> > > Hi,
> > > 
> > > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > > Works for me. The above needs the following fix folded in to build:
> > > > > > 
> > > > > > --- a/arch/arm/mm/proc-v7.S
> > > > > > +++ b/arch/arm/mm/proc-v7.S
> > > > > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > > > > >  __v7_ca8_proc_info:
> > > > > >  	.long	0x410fc080
> > > > > >  	.long	0xff0ffff0
> > > > > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > > > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > > > > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > > > > >  
> > > > > >  #endif	/* CONFIG_ARM_LPAE */
> > > > > 
> > > > > Thanks, merged into the original patch.
> > > > 
> > > > Do you want to give me an ack for this, thanks?
> > > 
> > > I tried to test this together with Tony's follow up patch, but I get
> > > this after applying the patch to v4.0:
> > > 
> > > sre@earth ~/src/linux [430973-fix] % make -j4
> > >   CHK     include/config/kernel.release
> > >   CHK     include/generated/uapi/linux/version.h
> > >   CHK     include/generated/utsrelease.h
> > > make[1]: 'include/generated/mach-types.h' is up to date.
> > >   CALL    scripts/checksyscalls.sh
> > >   CHK     include/generated/compile.h
> > >   AS      arch/arm/mm/proc-v7.o
> > > arch/arm/mm/proc-v7.S: Assembler messages:
> > > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > > scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
> > > make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
> > > Makefile:947: recipe for target 'arch/arm/mm' failed
> > > make: *** [arch/arm/mm] Error 2
> > > make: *** Waiting for unfinished jobs....
> > 
> > Maybe test the version in Linux next:
> > 
> > a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")
> 
> DONE with your your patch added on top:
> 
> Tested-By: Sebastian Reichel <[e-mail]>
> 
> (on N900)

OK thanks, patch now uploaded to Russell's patch system:

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8345/1
 
> I guess we should also drop the CONFIG_ARM_ERRATA_430973 check from
> pdata-quirks' nokia_n900_legacy_init() and just enable it unconditionally.

Yeah makes sense to me.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Russell King - ARM Linux April 23, 2015, 10:25 a.m. UTC | #7

On Mon, Apr 20, 2015 at 04:40:32PM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <[e-mail]> [150417 11:43]:
> > On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> > > * Sebastian Reichel <[e-mail]> [150415 09:32]:
> > > > Hi,
> > > > 
> > > > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > > > Works for me. The above needs the following fix folded in to build:
> > > > > > > 
> > > > > > > --- a/arch/arm/mm/proc-v7.S
> > > > > > > +++ b/arch/arm/mm/proc-v7.S
> > > > > > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > > > > > >  __v7_ca8_proc_info:
> > > > > > >  	.long	0x410fc080
> > > > > > >  	.long	0xff0ffff0
> > > > > > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > > > > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > > > > > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > > > > > >  
> > > > > > >  #endif	/* CONFIG_ARM_LPAE */
> > > > > > 
> > > > > > Thanks, merged into the original patch.
> > > > > 
> > > > > Do you want to give me an ack for this, thanks?
> > > > 
> > > > I tried to test this together with Tony's follow up patch, but I get
> > > > this after applying the patch to v4.0:
> > > > 
> > > > sre@earth ~/src/linux [430973-fix] % make -j4
> > > >   CHK     include/config/kernel.release
> > > >   CHK     include/generated/uapi/linux/version.h
> > > >   CHK     include/generated/utsrelease.h
> > > > make[1]: 'include/generated/mach-types.h' is up to date.
> > > >   CALL    scripts/checksyscalls.sh
> > > >   CHK     include/generated/compile.h
> > > >   AS      arch/arm/mm/proc-v7.o
> > > > arch/arm/mm/proc-v7.S: Assembler messages:
> > > > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > > > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > > > scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
> > > > make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
> > > > Makefile:947: recipe for target 'arch/arm/mm' failed
> > > > make: *** [arch/arm/mm] Error 2
> > > > make: *** Waiting for unfinished jobs....
> > > 
> > > Maybe test the version in Linux next:
> > > 
> > > a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")
> > 
> > DONE with your your patch added on top:
> > 
> > Tested-By: Sebastian Reichel <[e-mail]>
> > 
> > (on N900)
> 
> OK thanks, patch now uploaded to Russell's patch system:
> 
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8345/1

I have a concern with that patch.

The reason that it's disabled for multiplatform is because we can't
guarantee that the auxctrl register will be writable.  The solution
we came up with for multiplatform was to require firmware to be
updated to enable this bit.

Enabling it on a platform where firmware has not been updated, but
runs in the non-secure world will lead to the kernel hanging in the
early assembly code.

I've discussed it with Catalin, and Catalin's position is that we
should not remove the !multiplatform conditional.  That's something
which I find that I'm agreeing with Catalin on - any other non-secure
Cortex A8 user who is setting this bit in firmware will instantly
break if this patch is applied.

However, I don't think anyone is willing to say that they have a
solution to this problem - obviously, you can't build OMAP as a
non-multiplatform kernel anymore, so in effect you can never have
the kernel enable this errata.  And you can't detect whether you're
running in secure mode or not.

We could do the "only write the bit if it was originally clear" but
we still have the problem that doing so may cause other people
regressions.

Tony Lindgren April 23, 2015, 2:17 p.m. UTC | #8

* Russell King - ARM Linux <[e-mail]> [150423 03:26]:
> On Mon, Apr 20, 2015 at 04:40:32PM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <[e-mail]> [150417 11:43]:
> > > On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> > > > * Sebastian Reichel <[e-mail]> [150415 09:32]:
> > > > > Hi,
> > > > > 
> > > > > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > > > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > > > > Works for me. The above needs the following fix folded in to build:
> > > > > > > > 
> > > > > > > > --- a/arch/arm/mm/proc-v7.S
> > > > > > > > +++ b/arch/arm/mm/proc-v7.S
> > > > > > > > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
> > > > > > > >  __v7_ca8_proc_info:
> > > > > > > >  	.long	0x410fc080
> > > > > > > >  	.long	0xff0ffff0
> > > > > > > > -	__v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
> > > > > > > > +	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
> > > > > > > >  	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
> > > > > > > >  
> > > > > > > >  #endif	/* CONFIG_ARM_LPAE */
> > > > > > > 
> > > > > > > Thanks, merged into the original patch.
> > > > > > 
> > > > > > Do you want to give me an ack for this, thanks?
> > > > > 
> > > > > I tried to test this together with Tony's follow up patch, but I get
> > > > > this after applying the patch to v4.0:
> > > > > 
> > > > > sre@earth ~/src/linux [430973-fix] % make -j4
> > > > >   CHK     include/config/kernel.release
> > > > >   CHK     include/generated/uapi/linux/version.h
> > > > >   CHK     include/generated/utsrelease.h
> > > > > make[1]: 'include/generated/mach-types.h' is up to date.
> > > > >   CALL    scripts/checksyscalls.sh
> > > > >   CHK     include/generated/compile.h
> > > > >   AS      arch/arm/mm/proc-v7.o
> > > > > arch/arm/mm/proc-v7.S: Assembler messages:
> > > > > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > > > > arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
> > > > > scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
> > > > > make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
> > > > > Makefile:947: recipe for target 'arch/arm/mm' failed
> > > > > make: *** [arch/arm/mm] Error 2
> > > > > make: *** Waiting for unfinished jobs....
> > > > 
> > > > Maybe test the version in Linux next:
> > > > 
> > > > a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")
> > > 
> > > DONE with your your patch added on top:
> > > 
> > > Tested-By: Sebastian Reichel <[e-mail]>
> > > 
> > > (on N900)
> > 
> > OK thanks, patch now uploaded to Russell's patch system:
> > 
> > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8345/1
> 
> I have a concern with that patch.
> 
> The reason that it's disabled for multiplatform is because we can't
> guarantee that the auxctrl register will be writable.  The solution
> we came up with for multiplatform was to require firmware to be
> updated to enable this bit.
> 
> Enabling it on a platform where firmware has not been updated, but
> runs in the non-secure world will lead to the kernel hanging in the
> early assembly code.
> 
> I've discussed it with Catalin, and Catalin's position is that we
> should not remove the !multiplatform conditional.  That's something
> which I find that I'm agreeing with Catalin on - any other non-secure
> Cortex A8 user who is setting this bit in firmware will instantly
> break if this patch is applied.
> 
> However, I don't think anyone is willing to say that they have a
> solution to this problem - obviously, you can't build OMAP as a
> non-multiplatform kernel anymore, so in effect you can never have
> the kernel enable this errata.  And you can't detect whether you're
> running in secure mode or not.
> 
> We could do the "only write the bit if it was originally clear" but
> we still have the problem that doing so may cause other people
> regressions.

How about we keep the bit writing part !multiplatform conditional
(or even remove it) but always do the flush for ca-8?

Then we could also do a warning for a misconfigured ca-8 later on.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Matthijs van Duin April 24, 2015, 8:54 a.m. UTC | #9

On 23 April 2015 at 12:25, Russell King - ARM Linux
<[e-mail]> wrote:
> And you can't detect whether you're running in secure mode or not.

If not, you get an undefined instruction exception, which you could trap.

This may not be convenient, but "can't detect" is an overstatement.

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Russell King - ARM Linux April 28, 2015, 6:11 p.m. UTC | #10

On Fri, Apr 24, 2015 at 10:54:29AM +0200, Matthijs van Duin wrote:
> On 23 April 2015 at 12:25, Russell King - ARM Linux
> <[e-mail]> wrote:
> > And you can't detect whether you're running in secure mode or not.
> 
> If not, you get an undefined instruction exception, which you could trap.
> 
> This may not be convenient, but "can't detect" is an overstatement.

It's these kinds of statements that really piss me off.

At this stage in the boot, there's no memory allocators.  There's no
MMU.  There's really not very much.  There's no guarantee that the
location where the vectors are is writable on all platforms.

It's pretty much _impossible_ to do generically.

"Can't detect" is _not_ an overstatement.  It's a statement that I'm
giving you through my experience and knowledge of the Linux kernel,
the ARM archtecture, the capabilities of the platforms we have to
deal with, and how we want the kernel to work.

Sure, we _can_ detect it if (and only if) we code specifically for a
platform which has RAM at the CPU vector location.  Unfortunately,
that's a _very_ small proportion which approximates a number very
close to zero.

Russell King - ARM Linux April 28, 2015, 6:13 p.m. UTC | #11

On Thu, Apr 23, 2015 at 07:17:28AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <[e-mail]> [150423 03:26]:
> > However, I don't think anyone is willing to say that they have a
> > solution to this problem - obviously, you can't build OMAP as a
> > non-multiplatform kernel anymore, so in effect you can never have
> > the kernel enable this errata.  And you can't detect whether you're
> > running in secure mode or not.
> > 
> > We could do the "only write the bit if it was originally clear" but
> > we still have the problem that doing so may cause other people
> > regressions.
> 
> How about we keep the bit writing part !multiplatform conditional
> (or even remove it) but always do the flush for ca-8?
> 
> Then we could also do a warning for a misconfigured ca-8 later on.

Yes, we could do this - we'll have to rely on the boot loader doing
the right thing and setting this bit appropriately.  For those
platforms where this doesn't apply, I don't see any solution as (iirc)
OMAP now requires MULTIPLATFORM to be enabled.

Matthijs van Duin May 2, 2015, 6:51 a.m. UTC | #12

On 28 April 2015 at 20:11, Russell King - ARM Linux
<[e-mail]> wrote:
> "Can't detect" is _not_ an overstatement.  It's a statement that I'm
> giving you through my experience and knowledge of the Linux kernel,
> the ARM archtecture, the capabilities of the platforms we have to
> deal with, and how we want the kernel to work.
>
> Sure, we _can_ detect it if (and only if) we code specifically for a
> platform which has RAM at the CPU vector location.  Unfortunately,
> that's a _very_ small proportion which approximates a number very
> close to zero.

Sorry, I clearly had never adequately appreciated how much of a luxury
it is to be targeting only a limited range of platforms, or the burden
of the constraints imposed by needing to support the bewildering
diversity out there.

However, your objection isn't quite valid either since any ARM cpu
supporting the security extensions also supports changing the vector
base address.

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index bc86be205c04..fa385140715f 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -37,15 +37,18 @@ 
  *	It is assumed that:
  *	- we are not using split page tables
  */
-ENTRY(cpu_v7_switch_mm)
+ENTRY(cpu_ca8_switch_mm)
 #ifdef CONFIG_MMU
 	mov	r2, #0
-	mmid	r1, r1				@ get mm->context.id
-	ALT_SMP(orr	r0, r0, #TTB_FLAGS_SMP)
-	ALT_UP(orr	r0, r0, #TTB_FLAGS_UP)
 #ifdef CONFIG_ARM_ERRATA_430973
 	mcr	p15, 0, r2, c7, c5, 6		@ flush BTAC/BTB
 #endif
+#endif
+ENTRY(cpu_v7_switch_mm)
+#ifdef CONFIG_MMU
+	mmid	r1, r1				@ get mm->context.id
+	ALT_SMP(orr	r0, r0, #TTB_FLAGS_SMP)
+	ALT_UP(orr	r0, r0, #TTB_FLAGS_UP)
 #ifdef CONFIG_PID_IN_CONTEXTIDR
 	mrc	p15, 0, r2, c13, c0, 1		@ read current context ID
 	lsr	r2, r2, #8			@ extract the PID
@@ -61,6 +64,7 @@  ENTRY(cpu_v7_switch_mm)
 #endif
 	bx	lr
 ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_ca8_switch_mm)
 
 /*
  *	cpu_v7_set_pte_ext(ptep, pte)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index b1d19ea5e1af..003190ae9cd8 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -153,6 +153,21 @@  ENDPROC(cpu_v7_do_resume)
 #endif
 
 /*
+ * Cortex-A8
+ */
+	globl_equ	cpu_ca8_proc_init,	cpu_v7_proc_init
+	globl_equ	cpu_ca8_proc_fin,	cpu_v7_proc_fin
+	globl_equ	cpu_ca8_reset,		cpu_v7_reset
+	globl_equ	cpu_ca8_do_idle,	cpu_v7_do_idle
+	globl_equ	cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
+	globl_equ	cpu_ca8_set_pte_ext,	cpu_v7_set_pte_ext
+	globl_equ	cpu_ca8_suspend_size,	cpu_v7_suspend_size
+#ifdef CONFIG_ARM_CPU_SUSPEND
+	globl_equ	cpu_ca8_do_suspend,	cpu_v7_do_suspend
+	globl_equ	cpu_ca8_do_resume,	cpu_v7_do_resume
+#endif
+
+/*
  * Cortex-A9 processor functions
  */
 	globl_equ	cpu_ca9mp_proc_init,	cpu_v7_proc_init
@@ -471,7 +486,10 @@  __v7_setup_stack:
 
 	@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
 	define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#ifndef CONFIG_ARM_LPAE
+	define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 	define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#endif
 #ifdef CONFIG_CPU_PJ4B
 	define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
 #endif
@@ -527,6 +545,16 @@  __v7_ca9mp_proc_info:
 	__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
 	.size	__v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info
 
+	/*
+	 * ARM Ltd. Cortex A8 processor.
+	 */
+	.type	__v7_ca8_proc_info, #object
+__v7_ca8_proc_info:
+	.long	0x410fc080
+	.long	0xff0ffff0
+	__v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
+	.size	__v7_ca8_proc_info, . - __v7_ca8_proc_info
+
 #endif	/* CONFIG_ARM_LPAE */
 
 	/*