Re: [Xen-devel] [v3 03/15] Add cmpxchg16b support for x86-64

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

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx
> [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Andrew Cooper
> Sent: Thursday, June 25, 2015 2:35 AM
> To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> Cc: george.dunlap@xxxxxxxxxxxxx; Zhang, Yang Z; Tian, Kevin; keir@xxxxxxx;
> jbeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [v3 03/15] Add cmpxchg16b support for x86-64
> 
> On 24/06/15 06:18, Feng Wu wrote:
> > This patch adds cmpxchg16b support for x86-64, so software
> > can perform 128-bit atomic write/read.
> >
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > ---
> > v3:
> > Newly added.
> >
> >  xen/include/asm-x86/x86_64/system.h | 28
> ++++++++++++++++++++++++++++
> >  xen/include/xen/types.h             |  5 +++++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/xen/include/asm-x86/x86_64/system.h
> b/xen/include/asm-x86/x86_64/system.h
> > index 662813a..a910d00 100644
> > --- a/xen/include/asm-x86/x86_64/system.h
> > +++ b/xen/include/asm-x86/x86_64/system.h
> > @@ -6,6 +6,34 @@
> >                                     (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 ture.
> > + */
> > +
> > +static always_inline uint128_t __cmpxchg16b(
> > +    volatile void *ptr, uint128_t old, uint128_t new)
> 
> It is not nice for register scheduling taking uint128_t's by value.
> Instead, I would pass them by pointer and let the inlining sort the
> eventual references out.
> 
> > +{
> > +    uint128_t prev;
> > +
> > +    ASSERT(cpu_has_cx16);
> 
> Given that if this assertion were to fail, cmpxchg16b would fail with
> #UD, I would hand-code a asm_fixup section which in turn panics.  This
> avoids a situation where non-debug builds could die with an unqualified
> #UD exception.

Is there an existing way to panic the hypervisor in assembler code, I
don't find it, it would be appreciated if you can point it out.

> 
> Also, you must enforce 16-byte alignment of the memory reference, as
> described in the manual.

What should I do if the caller passes an non 16-byte alignment data
(struct iremap_entry in this case) ? Do this mean I need to define
it like this?

struct iremap_entry {

......

} __attribute__ ((aligned (16)));

Thanks,
Feng

> 
> ~Andrew
> 
> > +
> > +    asm volatile ( "lock; cmpxchg16b %4"
> > +                   : "=d" (prev.high), "=a" (prev.low)
> > +                   : "c" (new.high), "b" (new.low),
> > +                   "m" (*__xg((volatile void *)ptr)),
> > +                   "0" (old.high), "1" (old.low)
> > +                   : "memory" );
> > +
> > +    return prev;
> > +}
> > +
> > +#define cmpxchg16b(ptr,o,n)
> \
> > +    __cmpxchg16b((ptr), *(uint128_t *)(o), *(uint128_t *)(n))
> > +
> > +/*
> >   * This function causes value _o to be changed to _n at location _p.
> >   * If this access causes a fault then we return 1, otherwise we return 0.
> >   * If no fault occurs then _o is updated to the value we saw at _p. If this
> > diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
> > index 8596ded..30f8a44 100644
> > --- a/xen/include/xen/types.h
> > +++ b/xen/include/xen/types.h
> > @@ -47,6 +47,11 @@ typedef         __u64           uint64_t;
> >  typedef         __u64           u_int64_t;
> >  typedef         __s64           int64_t;
> >
> > +typedef struct {
> > +        uint64_t low;
> > +        uint64_t high;
> > +} uint128_t;
> > +
> >  struct domain;
> >  struct vcpu;
> >
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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