| Message ID | 20200502160700.19573-3-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rework {read, write}_atomic() | expand |
On Sat, 2 May 2020, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > The current implementation of write_atomic has two issues: > 1) It cannot be used to write pointer value because the switch > contains cast to other size than the size of the pointer. > 2) It will happily allow to write to a pointer to const. > > Additionally, the Arm implementation is returning a value when the x86 > implementation does not anymore. This was introduced in commit > 2934148a0773 "x86: simplify a few macros / inline functions". There are > no users of the return value, so it is fine to drop it. > > The switch is now moved in a static inline helper allowing the compiler > to prevent use of const pointer and also allow to write pointer value. > > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/include/asm-arm/atomic.h | 40 ++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 11 deletions(-) > > diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h > index 3c3d6bb04ee8..ac2798d095eb 100644 > --- a/xen/include/asm-arm/atomic.h > +++ b/xen/include/asm-arm/atomic.h > @@ -98,23 +98,41 @@ static always_inline void read_atomic_size(const volatile void *p, > } > } > > +static always_inline void write_atomic_size(volatile void *p, > + void *val, > + unsigned int size) > +{ > + switch ( size ) > + { > + case 1: > + write_u8_atomic(p, *(uint8_t *)val); > + break; > + case 2: > + write_u16_atomic(p, *(uint16_t *)val); > + break; > + case 4: > + write_u32_atomic(p, *(uint32_t *)val); > + break; > + case 8: > + write_u64_atomic(p, *(uint64_t *)val); > + break; > + default: > + __bad_atomic_size(); > + break; > + } > +} > + > #define read_atomic(p) ({ \ > union { typeof(*p) val; char c[0]; } x_; \ > read_atomic_size(p, x_.c, sizeof(*p)); \ > x_.val; \ > }) > > -#define write_atomic(p, x) ({ \ > - typeof(*p) __x = (x); \ > - switch ( sizeof(*p) ) { \ > - case 1: write_u8_atomic((uint8_t *)p, (uint8_t)__x); break; \ > - case 2: write_u16_atomic((uint16_t *)p, (uint16_t)__x); break; \ > - case 4: write_u32_atomic((uint32_t *)p, (uint32_t)__x); break; \ > - case 8: write_u64_atomic((uint64_t *)p, (uint64_t)__x); break; \ > - default: __bad_atomic_size(); break; \ > - } \ > - __x; \ > -}) > +#define write_atomic(p, x) \ > + do { \ > + typeof(*p) x_ = (x); \ > + write_atomic_size(p, &x_, sizeof(*p)); \ > + } while ( false ) > > #define add_sized(p, x) ({ \ > typeof(*(p)) __x = (x); \ > -- > 2.17.1 >
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h index 3c3d6bb04ee8..ac2798d095eb 100644 --- a/xen/include/asm-arm/atomic.h +++ b/xen/include/asm-arm/atomic.h @@ -98,23 +98,41 @@ static always_inline void read_atomic_size(const volatile void *p, } } +static always_inline void write_atomic_size(volatile void *p, + void *val, + unsigned int size) +{ + switch ( size ) + { + case 1: + write_u8_atomic(p, *(uint8_t *)val); + break; + case 2: + write_u16_atomic(p, *(uint16_t *)val); + break; + case 4: + write_u32_atomic(p, *(uint32_t *)val); + break; + case 8: + write_u64_atomic(p, *(uint64_t *)val); + break; + default: + __bad_atomic_size(); + break; + } +} + #define read_atomic(p) ({ \ union { typeof(*p) val; char c[0]; } x_; \ read_atomic_size(p, x_.c, sizeof(*p)); \ x_.val; \ }) -#define write_atomic(p, x) ({ \ - typeof(*p) __x = (x); \ - switch ( sizeof(*p) ) { \ - case 1: write_u8_atomic((uint8_t *)p, (uint8_t)__x); break; \ - case 2: write_u16_atomic((uint16_t *)p, (uint16_t)__x); break; \ - case 4: write_u32_atomic((uint32_t *)p, (uint32_t)__x); break; \ - case 8: write_u64_atomic((uint64_t *)p, (uint64_t)__x); break; \ - default: __bad_atomic_size(); break; \ - } \ - __x; \ -}) +#define write_atomic(p, x) \ + do { \ + typeof(*p) x_ = (x); \ + write_atomic_size(p, &x_, sizeof(*p)); \ + } while ( false ) #define add_sized(p, x) ({ \ typeof(*(p)) __x = (x); \