From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53272) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1am0bH-0006Lv-SL for qemu-devel@nongnu.org; Fri, 01 Apr 2016 11:03:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1am0bD-0000e9-OT for qemu-devel@nongnu.org; Fri, 01 Apr 2016 11:03:03 -0400 Received: from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]:34520) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1am0bD-0000dx-D5 for qemu-devel@nongnu.org; Fri, 01 Apr 2016 11:02:59 -0400 Received: by mail-wm0-x234.google.com with SMTP id p65so29711575wmp.1 for ; Fri, 01 Apr 2016 08:02:59 -0700 (PDT) References: <1458577386-9984-1-git-send-email-alex.bennee@linaro.org> <1458577386-9984-3-git-send-email-alex.bennee@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1458577386-9984-3-git-send-email-alex.bennee@linaro.org> Date: Fri, 01 Apr 2016 16:03:01 +0100 Message-ID: <87pou9fk2i.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v1 2/2] include/qemu/atomic: add compile time asserts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: pbonzini@redhat.com, sbruno@freebsd.org, peter.maydell@linaro.org Alex Bennée writes: > To be safely portable no atomic access should be trying to do more than > the natural word width of the host. The most common abuse is trying to > atomically access 64 bit values on a 32 bit host. > > This patch adds some QEMU_BUILD_BUG_ON to the __atomic instrinsic paths > to create a build failure if (sizeof(*ptr) > sizeof(void *)). > > Signed-off-by: Alex Bennée Ping Paolo. Is this worth including in a re-spin? > --- > include/qemu/atomic.h | 58 ++++++++++++++++++++++++++++++--------------------- > 1 file changed, 34 insertions(+), 24 deletions(-) > > diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h > index 8f1d8d9..5bc4d6c 100644 > --- a/include/qemu/atomic.h > +++ b/include/qemu/atomic.h > @@ -42,30 +42,34 @@ > * loads/stores past the atomic operation load/store. However there is > * no explicit memory barrier for the processor. > */ > -#define atomic_read(ptr) \ > - ({ \ > - typeof(*ptr) _val; \ > - __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ > - _val; \ > +#define atomic_read(ptr) \ > + ({ \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > + typeof(*ptr) _val; \ > + __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ > + _val; \ > }) > > -#define atomic_set(ptr, i) do { \ > - typeof(*ptr) _val = (i); \ > - __atomic_store(ptr, &_val, __ATOMIC_RELAXED); \ > +#define atomic_set(ptr, i) do { \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > + typeof(*ptr) _val = (i); \ > + __atomic_store(ptr, &_val, __ATOMIC_RELAXED); \ > } while(0) > > /* Atomic RCU operations imply weak memory barriers */ > > -#define atomic_rcu_read(ptr) \ > - ({ \ > - typeof(*ptr) _val; \ > - __atomic_load(ptr, &_val, __ATOMIC_CONSUME); \ > - _val; \ > +#define atomic_rcu_read(ptr) \ > + ({ \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > + typeof(*ptr) _val; \ > + __atomic_load(ptr, &_val, __ATOMIC_CONSUME); \ > + _val; \ > }) > > -#define atomic_rcu_set(ptr, i) do { \ > - typeof(*ptr) _val = (i); \ > - __atomic_store(ptr, &_val, __ATOMIC_RELEASE); \ > +#define atomic_rcu_set(ptr, i) do { \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > + typeof(*ptr) _val = (i); \ > + __atomic_store(ptr, &_val, __ATOMIC_RELEASE); \ > } while(0) > > /* atomic_mb_read/set semantics map Java volatile variables. They are > @@ -79,6 +83,7 @@ > #if defined(_ARCH_PPC) > #define atomic_mb_read(ptr) \ > ({ \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > typeof(*ptr) _val; \ > __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ > smp_rmb(); \ > @@ -86,22 +91,25 @@ > }) > > #define atomic_mb_set(ptr, i) do { \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > typeof(*ptr) _val = (i); \ > smp_wmb(); \ > __atomic_store(ptr, &_val, __ATOMIC_RELAXED); \ > smp_mb(); \ > } while(0) > #else > -#define atomic_mb_read(ptr) \ > - ({ \ > - typeof(*ptr) _val; \ > - __atomic_load(ptr, &_val, __ATOMIC_SEQ_CST); \ > - _val; \ > +#define atomic_mb_read(ptr) \ > + ({ \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > + typeof(*ptr) _val; \ > + __atomic_load(ptr, &_val, __ATOMIC_SEQ_CST); \ > + _val; \ > }) > > -#define atomic_mb_set(ptr, i) do { \ > - typeof(*ptr) _val = (i); \ > - __atomic_store(ptr, &_val, __ATOMIC_SEQ_CST); \ > +#define atomic_mb_set(ptr, i) do { \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > + typeof(*ptr) _val = (i); \ > + __atomic_store(ptr, &_val, __ATOMIC_SEQ_CST); \ > } while(0) > #endif > > @@ -109,6 +117,7 @@ > /* All the remaining operations are fully sequentially consistent */ > > #define atomic_xchg(ptr, i) ({ \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > typeof(*ptr) _new = (i), _old; \ > __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \ > _old; \ > @@ -117,6 +126,7 @@ > /* Returns the eventual value, failed or not */ > #define atomic_cmpxchg(ptr, old, new) \ > ({ \ > + QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ > typeof(*ptr) _old = (old), _new = (new); \ > __atomic_compare_exchange(ptr, &_old, &_new, false, \ > __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \ -- Alex Bennée