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: Wednesday, October 14, 2015 5:06 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 14.10.15 at 07:57, <feng.wu@xxxxxxxxx> wrote:
> 
> >
> >> -----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.
> 
> Did you misunderstand my reply? I'm not asking the memory clobber
> to be re-added (after all it was me who asked for it to be removed).
> Instead I'm pointing out that with the removal of the clobber the
> memory operand needs to become an input and output one, instead
> of just an input (which would make the compiler believe the memory
> location's contents don't change).

I understand your comments above. My understanding about the gcc
online doc is we don't need to add memory clobber when the variable
is either an input _or_ output one. However, seems your options is
memory clobber can be removed only when it is an input _and_ output
one. It would be highly appreciated if you can correct me if I understand
the doc incorrectly.

Thanks,
Feng 

> 
> Jan


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