diff mbox

ARM: mvebu: Fix bug in coherency fabric low level init function

Message ID 1369299242-16506-2-git-send-email-gregory.clement@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gregory CLEMENT May 23, 2013, 8:54 a.m. UTC
From: Nadav Haklai <nadavh@marvell.com>

When adding CPU to the SMP group and enabling the coherency on this
CPU we must protect the register access.
The previous implementation claims to be atomic but doesn't provide
any protection against parallel access to the coherency fabric control
and configuration registers.

This patch fixes this by using the ldrex and strex mechanism.
This method should be used in all accesses to those registers.

[gregory.clement@free-electrons.com: fixed the commit's topic]
Signed-off-by: Nadav Haklai <nadavh@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency_ll.S | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Jason Cooper May 23, 2013, 5:42 p.m. UTC | #1
On Thu, May 23, 2013 at 10:54:02AM +0200, Gregory CLEMENT wrote:
> From: Nadav Haklai <nadavh@marvell.com>
> 
> When adding CPU to the SMP group and enabling the coherency on this
> CPU we must protect the register access.
> The previous implementation claims to be atomic but doesn't provide
> any protection against parallel access to the coherency fabric control
> and configuration registers.
> 
> This patch fixes this by using the ldrex and strex mechanism.
> This method should be used in all accesses to those registers.
> 
> [gregory.clement@free-electrons.com: fixed the commit's topic]
> Signed-off-by: Nadav Haklai <nadavh@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/coherency_ll.S | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)

Applied to mvebu/fixes

thx,

Jason.
Willy Tarreau May 23, 2013, 6:15 p.m. UTC | #2
Hi Gregory,

On Thu, May 23, 2013 at 10:54:02AM +0200, Gregory CLEMENT wrote:
> From: Nadav Haklai <nadavh@marvell.com>
> 
> When adding CPU to the SMP group and enabling the coherency on this
> CPU we must protect the register access.
> The previous implementation claims to be atomic but doesn't provide
> any protection against parallel access to the coherency fabric control
> and configuration registers.
> 
> This patch fixes this by using the ldrex and strex mechanism.
> This method should be used in all accesses to those registers.

I don't know how to tell whether the fix works since it's unclear
to me what issue it fixes, but at least I can say that it doesn't
break my armada370 on 3.10-rc2 + latest fixes from your dev branch.

Best regards,
Willy
diff mbox

Patch

diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
index 53e8391..5476669 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -32,15 +32,21 @@  ENTRY(ll_set_cpu_coherent)
 
 	/* Add CPU to SMP group - Atomic */
 	add	r3, r0, #ARMADA_XP_CFB_CTL_REG_OFFSET
-	ldr	r2, [r3]
+1:
+	ldrex	r2, [r3]
 	orr	r2, r2, r1
-	str	r2, [r3]
+	strex 	r0, r2, [r3]
+	cmp	r0, #0
+	bne 1b
 
 	/* Enable coherency on CPU - Atomic */
-	add	r3, r0, #ARMADA_XP_CFB_CFG_REG_OFFSET
-	ldr	r2, [r3]
+	add	r3, r3, #ARMADA_XP_CFB_CFG_REG_OFFSET
+1:
+	ldrex	r2, [r3]
 	orr	r2, r2, r1
-	str	r2, [r3]
+	strex	r0, r2, [r3]
+	cmp	r0, #0
+	bne 1b
 
 	dsb