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

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Tuesday, October 13, 2015 11:29 PM
> To: Wu, Feng <feng.wu@xxxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxx;
> Keir Fraser <keir@xxxxxxx>
> Subject: Re: [PATCH v8 02/17] Add cmpxchg16b support for x86-64
> 
> >>> On 12.10.15 at 10:54, <feng.wu@xxxxxxxxx> wrote:
> > --- a/xen/include/asm-x86/x86_64/system.h
> > +++ b/xen/include/asm-x86/x86_64/system.h
> > @@ -6,6 +6,39 @@
> >                                     (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 true.
> > + */
> > +
> > +static always_inline __uint128_t __cmpxchg16b(
> > +    volatile void *ptr, const __uint128_t *old, const __uint128_t *new)
> > +{
> > +    __uint128_t prev;
> > +    uint64_t new_high = *new >> 64;
> > +    uint64_t new_low = *new;
> > +
> > +    ASSERT(cpu_has_cx16);
> > +
> > +    asm volatile ( "lock; cmpxchg16b %3"
> > +                   : "=A" (prev)
> > +                   : "c" (new_high), "b" (new_low),
> > +                     "m" (*__xg(ptr)), "0" (*old) );
> 
> I was about to apply this when I spotted that this is still wrong:
> The (requested) removal of the memory clobber requires that
> the memory location (all 16 bytes of it) appear as output. 

This is the description about memory clobber from gcc online docs:
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Clobbers

"The "memory" clobber tells the compiler that the assembly code
performs memory reads or writes to items other than those listed
in the input and output operands"

My understanding to it is if the variable is listed as input of output,
we don't need to add memory clobber. We need to add it only
when it is not listed in the intput/output and the memory may be
changed behind us.

What is your understanding about it? Thanks a lot!

Thanks,
Feng

> With __xg() covering 100 bytes, it should be possible to simply make
> the operand an in- and output ("+m") instead of just an input.
> 
> Jan


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