Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect
@ 2013-06-06 13:58 Jon Medhurst (Tixy)
  2013-06-06 15:01 ` Will Deacon
  0 siblings, 1 reply; 5+ messages in thread
From: Jon Medhurst (Tixy) @ 2013-06-06 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Cortex-A9 before version r1p0, the LoUIS bit field of the CLIDR
register returns zero when it should return one. This leads to cache
maintenance operations which rely on this value to not function as
intended, causing data corruption.

The workaround for this errata is to detect affected CPUs and correct
the LoUIS value read.

We also selected this workaround in CONFIG_ARCH_VEXPRESS_CA9X4 as that
platform is affected by the errata and as a consequence was suffering
reboot and shutdown crashes since the cpu hotplug changes introduced in
Linux 3.10. (Commit bca7a5a04933 "ARM: cpu hotplug: remove majority of
cache flushing from platforms")

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/Kconfig               |   10 ++++++++++
 arch/arm/mach-vexpress/Kconfig |    1 +
 arch/arm/mm/cache-v7.S         |    8 ++++++++
 3 files changed, 19 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 49d993c..1cd577c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1189,6 +1189,16 @@ config PL310_ERRATA_588369
 	   is not correctly implemented in PL310 as clean lines are not
 	   invalidated as a result of these operations.
 
+config ARM_ERRATA_643719
+	bool "ARM errata: LoUIS bit field in CLIDR register is incorrect"
+	depends on CPU_V7
+	help
+	  This option enables the workaround for the 643719 Cortex-A9 (prior to
+	  r1p0) erratum. On affected cores the LoUIS bit field of the CLIDR
+	  register returns zero when it should return one. The workaround
+	  corrects this value, ensuring cache maintenance operations which use
+	  it behave as intended and avoiding data corruption.
+
 config ARM_ERRATA_720789
 	bool "ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID"
 	depends on CPU_V7
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 5907e10..fe637e9 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -56,5 +56,6 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
 
 config ARCH_VEXPRESS_CA9X4
 	bool "Versatile Express Cortex-A9x4 tile"
+	select ARM_ERRATA_643719
 
 endmenu
diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
index 15451ee..05993ba 100644
--- a/arch/arm/mm/cache-v7.S
+++ b/arch/arm/mm/cache-v7.S
@@ -92,6 +92,14 @@ ENTRY(v7_flush_dcache_louis)
 	mrc	p15, 1, r0, c0, c0, 1		@ read clidr, r0 = clidr
 	ALT_SMP(ands	r3, r0, #(7 << 21))	@ extract LoUIS from clidr
 	ALT_UP(ands	r3, r0, #(7 << 27))	@ extract LoUU from clidr
+#ifdef CONFIG_ARM_ERRATA_643719
+	ALT_SMP(mrceq	p15, 0, r2, c0, c0, 0)	@ read main ID register
+	ALT_UP(moveq	pc, lr)			@ LoUU is zero, so nothing to do
+	biceq	r2, r2, #0x0000000f             @ clear minor revision number
+	ldreq	r1, =0x410fc090                 @ ID of ARM Cortex A9 r0p?
+	teqeq	r2, r1                          @ test for errata affected core and if so...
+	orreqs	r3, #(1 << 21)			@   fix LoUIS value (and set flags state to 'ne')
+#endif
 	ALT_SMP(mov	r3, r3, lsr #20)	@ r3 = LoUIS * 2
 	ALT_UP(mov	r3, r3, lsr #26)	@ r3 = LoUU * 2
 	moveq	pc, lr				@ return if level == 0
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect
  2013-06-06 13:58 [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect Jon Medhurst (Tixy)
@ 2013-06-06 15:01 ` Will Deacon
  2013-06-06 15:34   ` Jon Medhurst (Tixy)
  0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2013-06-06 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jun 06, 2013 at 02:58:32PM +0100, Jon Medhurst (Tixy) wrote:
> On Cortex-A9 before version r1p0, the LoUIS bit field of the CLIDR
> register returns zero when it should return one. This leads to cache
> maintenance operations which rely on this value to not function as
> intended, causing data corruption.
> 
> The workaround for this errata is to detect affected CPUs and correct
> the LoUIS value read.
> 
> We also selected this workaround in CONFIG_ARCH_VEXPRESS_CA9X4 as that
> platform is affected by the errata and as a consequence was suffering
> reboot and shutdown crashes since the cpu hotplug changes introduced in
> Linux 3.10. (Commit bca7a5a04933 "ARM: cpu hotplug: remove majority of
> cache flushing from platforms")
> 
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
>  arch/arm/Kconfig               |   10 ++++++++++
>  arch/arm/mach-vexpress/Kconfig |    1 +
>  arch/arm/mm/cache-v7.S         |    8 ++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 49d993c..1cd577c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1189,6 +1189,16 @@ config PL310_ERRATA_588369
>  	   is not correctly implemented in PL310 as clean lines are not
>  	   invalidated as a result of these operations.
>  
> +config ARM_ERRATA_643719
> +	bool "ARM errata: LoUIS bit field in CLIDR register is incorrect"
> +	depends on CPU_V7

You could add an SMP dependency here too.

> +	help
> +	  This option enables the workaround for the 643719 Cortex-A9 (prior to
> +	  r1p0) erratum. On affected cores the LoUIS bit field of the CLIDR
> +	  register returns zero when it should return one. The workaround
> +	  corrects this value, ensuring cache maintenance operations which use
> +	  it behave as intended and avoiding data corruption.
> +
>  config ARM_ERRATA_720789
>  	bool "ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID"
>  	depends on CPU_V7
> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index 5907e10..fe637e9 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -56,5 +56,6 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
>  
>  config ARCH_VEXPRESS_CA9X4
>  	bool "Versatile Express Cortex-A9x4 tile"
> +	select ARM_ERRATA_643719

We don't do this for any other workarounds, so I'd suggest either dropping
this or having a separate patch adding all of those that are required.

>  endmenu
> diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> index 15451ee..05993ba 100644
> --- a/arch/arm/mm/cache-v7.S
> +++ b/arch/arm/mm/cache-v7.S
> @@ -92,6 +92,14 @@ ENTRY(v7_flush_dcache_louis)
>  	mrc	p15, 1, r0, c0, c0, 1		@ read clidr, r0 = clidr
>  	ALT_SMP(ands	r3, r0, #(7 << 21))	@ extract LoUIS from clidr
>  	ALT_UP(ands	r3, r0, #(7 << 27))	@ extract LoUU from clidr
> +#ifdef CONFIG_ARM_ERRATA_643719
> +	ALT_SMP(mrceq	p15, 0, r2, c0, c0, 0)	@ read main ID register
> +	ALT_UP(moveq	pc, lr)			@ LoUU is zero, so nothing to do
> +	biceq	r2, r2, #0x0000000f             @ clear minor revision number
> +	ldreq	r1, =0x410fc090                 @ ID of ARM Cortex A9 r0p?
> +	teqeq	r2, r1                          @ test for errata affected core and if so...
> +	orreqs	r3, #(1 << 21)			@   fix LoUIS value (and set flags state to 'ne')
> +#endif
>  	ALT_SMP(mov	r3, r3, lsr #20)	@ r3 = LoUIS * 2
>  	ALT_UP(mov	r3, r3, lsr #26)	@ r3 = LoUU * 2
>  	moveq	pc, lr				@ return if level == 0

This part looks good to me:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect
  2013-06-06 15:01 ` Will Deacon
@ 2013-06-06 15:34   ` Jon Medhurst (Tixy)
  2013-06-06 15:43     ` Will Deacon
  0 siblings, 1 reply; 5+ messages in thread
From: Jon Medhurst (Tixy) @ 2013-06-06 15:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2013-06-06 at 16:01 +0100, Will Deacon wrote:
> On Thu, Jun 06, 2013 at 02:58:32PM +0100, Jon Medhurst (Tixy) wrote:
> > On Cortex-A9 before version r1p0, the LoUIS bit field of the CLIDR
> > register returns zero when it should return one. This leads to cache
> > maintenance operations which rely on this value to not function as
> > intended, causing data corruption.
> > 
> > The workaround for this errata is to detect affected CPUs and correct
> > the LoUIS value read.
> > 
> > We also selected this workaround in CONFIG_ARCH_VEXPRESS_CA9X4 as that
> > platform is affected by the errata and as a consequence was suffering
> > reboot and shutdown crashes since the cpu hotplug changes introduced in
> > Linux 3.10. (Commit bca7a5a04933 "ARM: cpu hotplug: remove majority of
> > cache flushing from platforms")
> > 
> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > ---
> >  arch/arm/Kconfig               |   10 ++++++++++
> >  arch/arm/mach-vexpress/Kconfig |    1 +
> >  arch/arm/mm/cache-v7.S         |    8 ++++++++
> >  3 files changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 49d993c..1cd577c 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1189,6 +1189,16 @@ config PL310_ERRATA_588369
> >  	   is not correctly implemented in PL310 as clean lines are not
> >  	   invalidated as a result of these operations.
> >  
> > +config ARM_ERRATA_643719
> > +	bool "ARM errata: LoUIS bit field in CLIDR register is incorrect"
> > +	depends on CPU_V7
> 
> You could add an SMP dependency here too.

OK.

> > +	help
> > +	  This option enables the workaround for the 643719 Cortex-A9 (prior to
> > +	  r1p0) erratum. On affected cores the LoUIS bit field of the CLIDR
> > +	  register returns zero when it should return one. The workaround
> > +	  corrects this value, ensuring cache maintenance operations which use
> > +	  it behave as intended and avoiding data corruption.
> > +
> >  config ARM_ERRATA_720789
> >  	bool "ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID"
> >  	depends on CPU_V7
> > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> > index 5907e10..fe637e9 100644
> > --- a/arch/arm/mach-vexpress/Kconfig
> > +++ b/arch/arm/mach-vexpress/Kconfig
> > @@ -56,5 +56,6 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
> >  
> >  config ARCH_VEXPRESS_CA9X4
> >  	bool "Versatile Express Cortex-A9x4 tile"
> > +	select ARM_ERRATA_643719
> 
> We don't do this for any other workarounds, so I'd suggest either dropping
> this or having a separate patch adding all of those that are required.

Not quite sure by what you mean by 'all of those that are required'. All
the errata required for vexpress? Or all platforms which require the
errata workaround?

If we don't enable the workaround in the Kconfig, then do we just add it
to the vexpress_defconfig and hope people notice? Or, as we have
ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA, we could select it in there?

Finally, whilst I'm asking questions ;-) I was hoping to fix a
regression that slipped into 3.10, is this late -rc material or will it
end up waiting for 3.11?

> 
> >  endmenu
> > diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S
> > index 15451ee..05993ba 100644
> > --- a/arch/arm/mm/cache-v7.S
> > +++ b/arch/arm/mm/cache-v7.S
> > @@ -92,6 +92,14 @@ ENTRY(v7_flush_dcache_louis)
> >  	mrc	p15, 1, r0, c0, c0, 1		@ read clidr, r0 = clidr
> >  	ALT_SMP(ands	r3, r0, #(7 << 21))	@ extract LoUIS from clidr
> >  	ALT_UP(ands	r3, r0, #(7 << 27))	@ extract LoUU from clidr
> > +#ifdef CONFIG_ARM_ERRATA_643719
> > +	ALT_SMP(mrceq	p15, 0, r2, c0, c0, 0)	@ read main ID register
> > +	ALT_UP(moveq	pc, lr)			@ LoUU is zero, so nothing to do
> > +	biceq	r2, r2, #0x0000000f             @ clear minor revision number
> > +	ldreq	r1, =0x410fc090                 @ ID of ARM Cortex A9 r0p?
> > +	teqeq	r2, r1                          @ test for errata affected core and if so...
> > +	orreqs	r3, #(1 << 21)			@   fix LoUIS value (and set flags state to 'ne')
> > +#endif
> >  	ALT_SMP(mov	r3, r3, lsr #20)	@ r3 = LoUIS * 2
> >  	ALT_UP(mov	r3, r3, lsr #26)	@ r3 = LoUU * 2
> >  	moveq	pc, lr				@ return if level == 0
> 
> This part looks good to me:
> 
>   Acked-by: Will Deacon <will.deacon@arm.com>

-- 
Tixy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect
  2013-06-06 15:34   ` Jon Medhurst (Tixy)
@ 2013-06-06 15:43     ` Will Deacon
  2013-06-06 16:46       ` Nicolas Pitre
  0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2013-06-06 15:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jun 06, 2013 at 04:34:48PM +0100, Jon Medhurst (Tixy) wrote:
> On Thu, 2013-06-06 at 16:01 +0100, Will Deacon wrote:
> > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> > > index 5907e10..fe637e9 100644
> > > --- a/arch/arm/mach-vexpress/Kconfig
> > > +++ b/arch/arm/mach-vexpress/Kconfig
> > > @@ -56,5 +56,6 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
> > >  
> > >  config ARCH_VEXPRESS_CA9X4
> > >  	bool "Versatile Express Cortex-A9x4 tile"
> > > +	select ARM_ERRATA_643719
> > 
> > We don't do this for any other workarounds, so I'd suggest either dropping
> > this or having a separate patch adding all of those that are required.
> 
> Not quite sure by what you mean by 'all of those that are required'. All
> the errata required for vexpress? Or all platforms which require the
> errata workaround?

I mean, the A9 coretile on vexpress also has other errata, for which we have
workarounds in the kernel. They aren't automatically selected (for example,
ARM_ERRATA_720789) so if you want to do this, it's worth selecting all or
none, rather than make it look like you're selecting all those that apply
when you're not.

> If we don't enable the workaround in the Kconfig, then do we just add it
> to the vexpress_defconfig and hope people notice? Or, as we have
> ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA, we could select it in there?

It doesn't affect A5, so I wouldn't put it there. The usual place is the
defconfig.

> Finally, whilst I'm asking questions ;-) I was hoping to fix a
> regression that slipped into 3.10, is this late -rc material or will it
> end up waiting for 3.11?

The fix to v7_flush_dcache_louis should go in for 3.10 and probably stable
as well. I don't think the vexpress kconfig stuff is as important.

Will

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect
  2013-06-06 15:43     ` Will Deacon
@ 2013-06-06 16:46       ` Nicolas Pitre
  0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Pitre @ 2013-06-06 16:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 6 Jun 2013, Will Deacon wrote:

> On Thu, Jun 06, 2013 at 04:34:48PM +0100, Jon Medhurst (Tixy) wrote:
> > On Thu, 2013-06-06 at 16:01 +0100, Will Deacon wrote:
> > > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> > > > index 5907e10..fe637e9 100644
> > > > --- a/arch/arm/mach-vexpress/Kconfig
> > > > +++ b/arch/arm/mach-vexpress/Kconfig
> > > > @@ -56,5 +56,6 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
> > > >  
> > > >  config ARCH_VEXPRESS_CA9X4
> > > >  	bool "Versatile Express Cortex-A9x4 tile"
> > > > +	select ARM_ERRATA_643719
> > > 
> > > We don't do this for any other workarounds, so I'd suggest either dropping
> > > this or having a separate patch adding all of those that are required.
> > 
> > Not quite sure by what you mean by 'all of those that are required'. All
> > the errata required for vexpress? Or all platforms which require the
> > errata workaround?
> 
> I mean, the A9 coretile on vexpress also has other errata, for which we have
> workarounds in the kernel. They aren't automatically selected (for example,
> ARM_ERRATA_720789) so if you want to do this, it's worth selecting all or
> none, rather than make it look like you're selecting all those that apply
> when you're not.

I think we _really_ should move towards selecting all the errata that 
apply on a per platform basis.  We simply cannot expect everyone to be 
aware of the actual errata list that apply for a given platform.  And as 
we move towards reducing the amount of defconfig files we need a 
reliable way to record which erratum applies to which platform.

Many platforms appear to do it already:

mach-bcm/Kconfig:       select ARM_ERRATA_754322
mach-bcm/Kconfig:       select ARM_ERRATA_764369 if SMP
mach-bcm2835/Kconfig:   select ARM_ERRATA_411920
mach-imx/Kconfig:       select ARM_ERRATA_754322
mach-imx/Kconfig:       select ARM_ERRATA_764369 if SMP
mach-imx/Kconfig:       select ARM_ERRATA_775420
mach-imx/Kconfig:       select PL310_ERRATA_588369 if CACHE_PL310
mach-imx/Kconfig:       select PL310_ERRATA_727915 if CACHE_PL310
mach-imx/Kconfig:       select PL310_ERRATA_769419 if CACHE_PL310
mach-omap2/Kconfig:     select ARM_ERRATA_720789
mach-omap2/Kconfig:     select PL310_ERRATA_588369
mach-omap2/Kconfig:     select PL310_ERRATA_727915
mach-omap2/Kconfig:     select ARM_ERRATA_754322
mach-omap2/Kconfig:     select ARM_ERRATA_775420
mach-tegra/Kconfig:     select ARM_ERRATA_720789
mach-tegra/Kconfig:     select ARM_ERRATA_754327 if SMP
mach-tegra/Kconfig:     select ARM_ERRATA_764369 if SMP
mach-tegra/Kconfig:     select PL310_ERRATA_727915 if CACHE_L2X0
mach-tegra/Kconfig:     select PL310_ERRATA_769419 if CACHE_L2X0
mach-tegra/Kconfig:     select ARM_ERRATA_754322
mach-tegra/Kconfig:     select ARM_ERRATA_764369 if SMP
mach-tegra/Kconfig:     select PL310_ERRATA_769419 if CACHE_L2X0
mach-ux500/Kconfig:     select ARM_ERRATA_754322
mach-ux500/Kconfig:     select ARM_ERRATA_764369 if SMP
mach-ux500/Kconfig:     select PL310_ERRATA_753970 if CACHE_PL310

So if someone knowledgeable about applicable errata for VExpress could 
do the same that would be great.  I agree that this should be a separate 
patch though.


Nicolas

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-06-06 16:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-06 13:58 [PATCH] ARM: errata: LoUIS bit field in CLIDR register is incorrect Jon Medhurst (Tixy)
2013-06-06 15:01 ` Will Deacon
2013-06-06 15:34   ` Jon Medhurst (Tixy)
2013-06-06 15:43     ` Will Deacon
2013-06-06 16:46       ` Nicolas Pitre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox