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

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

> -----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))

Thanks,
Feng

> 
> Jan


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