| Message ID | 20200730170721.23393-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: cmpxchg: Add missing memory barriers in __cmpxchg_mb_timeout() | expand |
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 >
> 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 > >
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) \