Re: [Xen-devel] [PATCH v6 02/18] Add cmpxchg16b support for x86-64

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Wu, Feng
> Sent: Sunday, September 06, 2015 2:07 PM
> To: Jan Beulich
> Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxx; Keir Fraser; Wu, Feng
> Subject: RE: [PATCH v6 02/18] Add cmpxchg16b support for x86-64
> 
> 
> 
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: Friday, September 04, 2015 10:23 PM
> > To: Wu, Feng
> > Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxx; Keir Fraser
> > Subject: Re: [PATCH v6 02/18] Add cmpxchg16b support for x86-64
> >
> > >>> On 25.08.15 at 03:57, <feng.wu@xxxxxxxxx> wrote:
> > > This patch adds cmpxchg16b support for x86-64, so software
> > > can perform 128-bit atomic write/read.
> > >
> > > CC: Keir Fraser <keir@xxxxxxx>
> > > CC: Jan Beulich <jbeulich@xxxxxxxx>
> > > CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > > ---
> > > v6:
> > > - Fix a typo
> > >
> > > v5:
> > > - Change back the parameters of __cmpxchg16b() to __uint128_t *
> > > - Remove pointless cast for 'ptr'
> > > - Remove pointless parentheses
> > > - Use A constraint for the output
> >
> > There are a few comments I had on v4 which neither have been
> > addressed verbally nor have got incorporated into the patch. See
> > http://lists.xenproject.org/archives/html/xen-devel/2015-07/msg04694.html
> 
> Do you mean I missed the following part in the above link?
> 
> " But I think now I recall that it may have been me asking for using
> void pointers here: That way one can pass pointers to other types.
> But then you would better cast to void * here, and have the inline
> function have properly typed pointers. And it should go without
> saying that if you cast (or implicitly convert to void), you should
> validate that what your caller handed you is at least the size of a
> __uint128_t (and for "ptr" perhaps also, as I think Andrew already
> pointed out, that it is suitably aligned). "
> 
> If that is the case, what about the changes below?
> 
> #define cmpxchg16b(ptr,o,n)
> \
>     ASSERT((unsigned long)ptr & 0xF == 0);
> \
>     ASSERT(sizeof(*o) == sizeof(__uint128_t))
> \
>     ASSERT(sizeof(*n) == sizeof(__uint128_t))
> \
>     __cmpxchg16b((ptr), (void *)(o), (void *)(n))

Seems there is a build error with this change, we cannot
add stuff before __cmpxchg() since it needs return some
value to the caller. Any suggestion here?

Thanks,
Feng


> 
> Thanks,
> Feng
> 
> >
> > Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel