linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for 3.10-rc] ARM: mvebu: Fix bug in coherency fabric low level init function
@ 2013-05-23  8:54 Gregory CLEMENT
  2013-05-23  8:54 ` [PATCH] " Gregory CLEMENT
  0 siblings, 1 reply; 4+ messages in thread
From: Gregory CLEMENT @ 2013-05-23  8:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Jason and Andrew,

This a new fix for 3.10-rc coming from Marvell. I wasn't impacted
(until now) by the bug fixed by this commit, but I trust Nadav with
his patch, and I didn't see any flaw in his code. Furthermore, I
applied and tested it and saw no regression.

So please applied it for 3.10-rc and we can also consider to applied
it to 3.8 and 3.9.

Thanks

Nadav Haklai (1):
  ARM: mvebu: Fix bug in coherency fabric low level init function

 arch/arm/mach-mvebu/coherency_ll.S | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

-- 
1.8.1.2

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

* [PATCH] ARM: mvebu: Fix bug in coherency fabric low level init function
  2013-05-23  8:54 [PATCH for 3.10-rc] ARM: mvebu: Fix bug in coherency fabric low level init function Gregory CLEMENT
@ 2013-05-23  8:54 ` Gregory CLEMENT
  2013-05-23 17:42   ` Jason Cooper
  2013-05-23 18:15   ` Willy Tarreau
  0 siblings, 2 replies; 4+ messages in thread
From: Gregory CLEMENT @ 2013-05-23  8:54 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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(-)

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
 
-- 
1.8.1.2

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

* [PATCH] ARM: mvebu: Fix bug in coherency fabric low level init function
  2013-05-23  8:54 ` [PATCH] " Gregory CLEMENT
@ 2013-05-23 17:42   ` Jason Cooper
  2013-05-23 18:15   ` Willy Tarreau
  1 sibling, 0 replies; 4+ messages in thread
From: Jason Cooper @ 2013-05-23 17:42 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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.

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

* [PATCH] ARM: mvebu: Fix bug in coherency fabric low level init function
  2013-05-23  8:54 ` [PATCH] " Gregory CLEMENT
  2013-05-23 17:42   ` Jason Cooper
@ 2013-05-23 18:15   ` Willy Tarreau
  1 sibling, 0 replies; 4+ messages in thread
From: Willy Tarreau @ 2013-05-23 18:15 UTC (permalink / raw)
  To: linux-arm-kernel

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

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

end of thread, other threads:[~2013-05-23 18:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-23  8:54 [PATCH for 3.10-rc] ARM: mvebu: Fix bug in coherency fabric low level init function Gregory CLEMENT
2013-05-23  8:54 ` [PATCH] " Gregory CLEMENT
2013-05-23 17:42   ` Jason Cooper
2013-05-23 18:15   ` Willy Tarreau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).