From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wu, Feng" Subject: Re: [v3 03/15] Add cmpxchg16b support for x86-64 Date: Wed, 8 Jul 2015 07:06:48 +0000 Message-ID: References: <1435123109-10481-1-git-send-email-feng.wu@intel.com> <1435123109-10481-4-git-send-email-feng.wu@intel.com> <558AF859.5020107@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <558AF859.5020107@citrix.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , "xen-devel@lists.xen.org" Cc: "Tian, Kevin" , "Wu, Feng" , "george.dunlap@eu.citrix.com" , "jbeulich@suse.com" , "Zhang, Yang Z" , "keir@xen.org" List-Id: xen-devel@lists.xenproject.org > -----Original Message----- > From: xen-devel-bounces@lists.xen.org > [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Andrew Cooper > Sent: Thursday, June 25, 2015 2:35 AM > To: Wu, Feng; xen-devel@lists.xen.org > Cc: george.dunlap@eu.citrix.com; Zhang, Yang Z; Tian, Kevin; keir@xen.org; > jbeulich@suse.com > Subject: Re: [Xen-devel] [v3 03/15] Add cmpxchg16b support for x86-64 > > On 24/06/15 06:18, Feng Wu wrote: > > This patch adds cmpxchg16b support for x86-64, so software > > can perform 128-bit atomic write/read. > > > > Signed-off-by: Feng Wu > > --- > > v3: > > Newly added. > > > > xen/include/asm-x86/x86_64/system.h | 28 > ++++++++++++++++++++++++++++ > > xen/include/xen/types.h | 5 +++++ > > 2 files changed, 33 insertions(+) > > > > diff --git a/xen/include/asm-x86/x86_64/system.h > b/xen/include/asm-x86/x86_64/system.h > > index 662813a..a910d00 100644 > > --- a/xen/include/asm-x86/x86_64/system.h > > +++ b/xen/include/asm-x86/x86_64/system.h > > @@ -6,6 +6,34 @@ > > (unsigned > long)(n),sizeof(*(ptr)))) > > > > /* > > + * Atomic 16 bytes compare and exchange. Compare OLD with MEM, if > > + * identical, store NEW in MEM. Return the initial value in MEM. > > + * Success is indicated by comparing RETURN with OLD. > > + * > > + * This function can only be called when cpu_has_cx16 is ture. > > + */ > > + > > +static always_inline uint128_t __cmpxchg16b( > > + volatile void *ptr, uint128_t old, uint128_t new) > > It is not nice for register scheduling taking uint128_t's by value. > Instead, I would pass them by pointer and let the inlining sort the > eventual references out. > > > +{ > > + uint128_t prev; > > + > > + ASSERT(cpu_has_cx16); > > Given that if this assertion were to fail, cmpxchg16b would fail with > #UD, I would hand-code a asm_fixup section which in turn panics. This > avoids a situation where non-debug builds could die with an unqualified > #UD exception. Is there an existing way to panic the hypervisor in assembler code, I don't find it, it would be appreciated if you can point it out. > > Also, you must enforce 16-byte alignment of the memory reference, as > described in the manual. What should I do if the caller passes an non 16-byte alignment data (struct iremap_entry in this case) ? Do this mean I need to define it like this? struct iremap_entry { ...... } __attribute__ ((aligned (16))); Thanks, Feng > > ~Andrew > > > + > > + asm volatile ( "lock; cmpxchg16b %4" > > + : "=d" (prev.high), "=a" (prev.low) > > + : "c" (new.high), "b" (new.low), > > + "m" (*__xg((volatile void *)ptr)), > > + "0" (old.high), "1" (old.low) > > + : "memory" ); > > + > > + return prev; > > +} > > + > > +#define cmpxchg16b(ptr,o,n) > \ > > + __cmpxchg16b((ptr), *(uint128_t *)(o), *(uint128_t *)(n)) > > + > > +/* > > * This function causes value _o to be changed to _n at location _p. > > * If this access causes a fault then we return 1, otherwise we return 0. > > * If no fault occurs then _o is updated to the value we saw at _p. If this > > diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h > > index 8596ded..30f8a44 100644 > > --- a/xen/include/xen/types.h > > +++ b/xen/include/xen/types.h > > @@ -47,6 +47,11 @@ typedef __u64 uint64_t; > > typedef __u64 u_int64_t; > > typedef __s64 int64_t; > > > > +typedef struct { > > + uint64_t low; > > + uint64_t high; > > +} uint128_t; > > + > > struct domain; > > struct vcpu; > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel