Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout()
@ 2020-07-30 17:07 Julien Grall
  2020-07-30 19:44 ` Stefano Stabellini
  2020-07-31 12:50 ` Bertrand Marquis
  0 siblings, 2 replies; 3+ messages in thread
From: Julien Grall @ 2020-07-30 17:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien

From: Julien Grall <jgrall@amazon.com>

The function __cmpxchg_mb_timeout() was intended to have the same
semantics as __cmpxchg_mb(). Unfortunately, the memory barriers were
not added when first implemented.

There is no known issue with the existing callers, but the barriers are
added given this is the expected semantics in Xen.

The issue was introduced by XSA-295.

Backport: 4.8+
Fixes: 86b0bc958373 ("xen/arm: cmpxchg: Provide a new helper that can timeout")
Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/include/asm-arm/arm32/cmpxchg.h | 8 +++++++-
 xen/include/asm-arm/arm64/cmpxchg.h | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/arm32/cmpxchg.h b/xen/include/asm-arm/arm32/cmpxchg.h
index 49ca2a0d7ab1..0770f272ee99 100644
--- a/xen/include/asm-arm/arm32/cmpxchg.h
+++ b/xen/include/asm-arm/arm32/cmpxchg.h
@@ -147,7 +147,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
 					       int size,
 					       unsigned int max_try)
 {
-	return __int_cmpxchg(ptr, old, new, size, true, max_try);
+	bool ret;
+
+	smp_mb();
+	ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
+	smp_mb();
+
+	return ret;
 }
 
 #define cmpxchg(ptr,o,n)						\
diff --git a/xen/include/asm-arm/arm64/cmpxchg.h b/xen/include/asm-arm/arm64/cmpxchg.h
index 5bc2e1f78674..fc5c60f0bd74 100644
--- a/xen/include/asm-arm/arm64/cmpxchg.h
+++ b/xen/include/asm-arm/arm64/cmpxchg.h
@@ -160,7 +160,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
 					       int size,
 					       unsigned int max_try)
 {
-	return __int_cmpxchg(ptr, old, new, size, true, max_try);
+	bool ret;
+
+	smp_mb();
+	ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
+	smp_mb();
+
+	return ret;
 }
 
 #define cmpxchg(ptr, o, n) \
-- 
2.17.1



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

* Re: [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout()
  2020-07-30 17:07 [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout() Julien Grall
@ 2020-07-30 19:44 ` Stefano Stabellini
  2020-07-31 12:50 ` Bertrand Marquis
  1 sibling, 0 replies; 3+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:44 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> The function __cmpxchg_mb_timeout() was intended to have the same
> semantics as __cmpxchg_mb(). Unfortunately, the memory barriers were
> not added when first implemented.
> 
> There is no known issue with the existing callers, but the barriers are
> added given this is the expected semantics in Xen.
> 
> The issue was introduced by XSA-295.
> 
> Backport: 4.8+
> Fixes: 86b0bc958373 ("xen/arm: cmpxchg: Provide a new helper that can timeout")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/include/asm-arm/arm32/cmpxchg.h | 8 +++++++-
>  xen/include/asm-arm/arm64/cmpxchg.h | 8 +++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/arm32/cmpxchg.h b/xen/include/asm-arm/arm32/cmpxchg.h
> index 49ca2a0d7ab1..0770f272ee99 100644
> --- a/xen/include/asm-arm/arm32/cmpxchg.h
> +++ b/xen/include/asm-arm/arm32/cmpxchg.h
> @@ -147,7 +147,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
>  					       int size,
>  					       unsigned int max_try)
>  {
> -	return __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	bool ret;
> +
> +	smp_mb();
> +	ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	smp_mb();
> +
> +	return ret;
>  }
>  
>  #define cmpxchg(ptr,o,n)						\
> diff --git a/xen/include/asm-arm/arm64/cmpxchg.h b/xen/include/asm-arm/arm64/cmpxchg.h
> index 5bc2e1f78674..fc5c60f0bd74 100644
> --- a/xen/include/asm-arm/arm64/cmpxchg.h
> +++ b/xen/include/asm-arm/arm64/cmpxchg.h
> @@ -160,7 +160,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
>  					       int size,
>  					       unsigned int max_try)
>  {
> -	return __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	bool ret;
> +
> +	smp_mb();
> +	ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	smp_mb();
> +
> +	return ret;
>  }
>  
>  #define cmpxchg(ptr, o, n) \
> -- 
> 2.17.1
> 


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

* Re: [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout()
  2020-07-30 17:07 [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout() Julien Grall
  2020-07-30 19:44 ` Stefano Stabellini
@ 2020-07-31 12:50 ` Bertrand Marquis
  1 sibling, 0 replies; 3+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:50 UTC (permalink / raw)
  To: Julien Grall
  Cc: Xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk, nd



> On 30 Jul 2020, at 19:07, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> The function __cmpxchg_mb_timeout() was intended to have the same
> semantics as __cmpxchg_mb(). Unfortunately, the memory barriers were
> not added when first implemented.
> 
> There is no known issue with the existing callers, but the barriers are
> added given this is the expected semantics in Xen.
> 
> The issue was introduced by XSA-295.
> 
> Backport: 4.8+
> Fixes: 86b0bc958373 ("xen/arm: cmpxchg: Provide a new helper that can timeout")
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

> ---
> xen/include/asm-arm/arm32/cmpxchg.h | 8 +++++++-
> xen/include/asm-arm/arm64/cmpxchg.h | 8 +++++++-
> 2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/arm32/cmpxchg.h b/xen/include/asm-arm/arm32/cmpxchg.h
> index 49ca2a0d7ab1..0770f272ee99 100644
> --- a/xen/include/asm-arm/arm32/cmpxchg.h
> +++ b/xen/include/asm-arm/arm32/cmpxchg.h
> @@ -147,7 +147,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
> 					       int size,
> 					       unsigned int max_try)
> {
> -	return __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	bool ret;
> +
> +	smp_mb();
> +	ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	smp_mb();
> +
> +	return ret;
> }
> 
> #define cmpxchg(ptr,o,n)						\
> diff --git a/xen/include/asm-arm/arm64/cmpxchg.h b/xen/include/asm-arm/arm64/cmpxchg.h
> index 5bc2e1f78674..fc5c60f0bd74 100644
> --- a/xen/include/asm-arm/arm64/cmpxchg.h
> +++ b/xen/include/asm-arm/arm64/cmpxchg.h
> @@ -160,7 +160,13 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
> 					       int size,
> 					       unsigned int max_try)
> {
> -	return __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	bool ret;
> +
> +	smp_mb();
> +	ret = __int_cmpxchg(ptr, old, new, size, true, max_try);
> +	smp_mb();
> +
> +	return ret;
> }
> 
> #define cmpxchg(ptr, o, n) \
> -- 
> 2.17.1
> 
> 



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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 17:07 [PATCH] xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout() Julien Grall
2020-07-30 19:44 ` Stefano Stabellini
2020-07-31 12:50 ` Bertrand Marquis

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git