[PATCH] ARM: errata: Remove SMP dependency for erratum 751472

[PATCH] ARM: errata: Remove SMP dependency for erratum 751472

[Dave Martin]

Activation conditions for a workaround should not be encoded in the
workaround's direct dependencies if this makes otherwise reasonable
configuration choices impossible.

This patches uses the SMP/UP patching facilities instead to compile
out the workaround if the configuration means that it is definitely
not needed.

This means that configs for buggy silicon can simply select
ARM_ERRATA_751472, without preventing a UP kernel from being built
or duplicatiing knowledge about when to activate the workaround.
This seems the correct why to do things, because the erratum is a
property of the silicon, irrespective of what the kernel config
happens to be.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/Kconfig      |    2 +-
 arch/arm/mm/proc-v7.S |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 44789ef..e5bbb2f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1281,7 +1281,7 @@ config ARM_ERRATA_743622
 
 config ARM_ERRATA_751472
 	bool "ARM errata: Interrupted ICIALLUIS may prevent completion of broadcasted operation"
-	depends on CPU_V7 && SMP
+	depends on CPU_V7
 	help
 	  This option enables the workaround for the 751472 Cortex-A9 (prior
 	  to r3p0) erratum. An interrupted ICIALLUIS operation may prevent the
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 2c559ac..9b06d0b 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -364,10 +364,12 @@ __v7_setup:
 	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register
 #endif
 #ifdef CONFIG_ARM_ERRATA_751472
-	cmp	r6, #0x30			@ present prior to r3p0
+	ALT_UP_B(1f)
+	ALT_SMP(cmp r6, #0x30)			@ present prior to r3p0
 	mrclt	p15, 0, r10, c15, c0, 1		@ read diagnostic register
 	orrlt	r10, r10, #1 << 11		@ set bit #11
 	mcrlt	p15, 0, r10, c15, c0, 1		@ write diagnostic register
+1:
 #endif
 
 3:	mov	r10, #0
-- 
1.7.4.1

[PATCH] ARM: errata: Remove SMP dependency for erratum 751472

[Russell King - ARM Linux]

On Tue, Nov 29, 2011 at 05:12:43PM +0000, Dave Martin wrote:
> -	cmp	r6, #0x30			@ present prior to r3p0
> +	ALT_UP_B(1f)
> +	ALT_SMP(cmp r6, #0x30)			@ present prior to r3p0

Why not look at the definition of these before you (ab)use them?

[PATCH] ARM: errata: Remove SMP dependency for erratum 751472

[Dave Martin]

On Tue, Nov 29, 2011 at 05:35:37PM +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 29, 2011 at 05:12:43PM +0000, Dave Martin wrote:
> > -	cmp	r6, #0x30			@ present prior to r3p0
> > +	ALT_UP_B(1f)
> > +	ALT_SMP(cmp r6, #0x30)			@ present prior to r3p0
> 
> Why not look at the definition of these before you (ab)use them?

Sorry -- I did look, but it looks like I confused myself afterwards, because
of the way the instruction to skip the SMP block has to be inserted in
between the first and second instructions of the block being skipped,
rather than at the start (as would happen in a conditional branch).

Do you have any concerns about that other than the ALT_SMP/UP being the
wrong way around?


One thing which strikes me about ALT_UP_B() is that the likely use is
to skip over the SMP case code (which may consist of many instructions).
This means that if the SMP code is not there at all (as in a UP kernel)
no branch is necessary.  But the definition is more conservative, so
that we're left with a branch jumping to the next instruction even in a
UP kernel.  The effect is that we really can branch to anywhere with
ALT_UP_B() -- a potentially useful feature, but one which we don't
currently appear to make use of.

An alternative would be for the ALT_UP_B() actually to disappear in a
UP kernel.  So far as I can see, such an implementation would be
compatible with all existing uses of this macro.  What do you think?

Cheers
---Dave

[PATCH] ARM: errata: Remove SMP dependency for erratum 751472

[Russell King - ARM Linux]

On Tue, Nov 29, 2011 at 06:06:18PM +0000, Dave Martin wrote:
> One thing which strikes me about ALT_UP_B() is that the likely use is
> to skip over the SMP case code (which may consist of many instructions).
> This means that if the SMP code is not there at all (as in a UP kernel)
> no branch is necessary.

It's there to solve the case in the IRQ entry code, where it's use is
already bounded by a #ifdef CONFIG_SMP.

> An alternative would be for the ALT_UP_B() actually to disappear in a
> UP kernel.  So far as I can see, such an implementation would be
> compatible with all existing uses of this macro.  What do you think?

That sounds dangerous, adding unexpected behaviour to the code.  For
example:

#ifdef CONFIG_SMP
        /*
         * XXX
         *
         * this macro assumes that irqstat (r2) and base (r6) are
         * preserved from get_irqnr_and_base above
         */
        ALT_SMP(test_for_ipi r0, r2, r6, lr)
        ALT_UP_B(9997f)
        movne   r1, sp
        adrne   lr, BSYM(1b)
        bne     do_IPI
#endif

If the surrounding ifdef goes away, and the ALT_UP_B() ends up being
optimized to nothing, then we no longer skip over this code.  So no,
it's not compatible with existing uses.