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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

Jan


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