>>> On 08.07.15 at 09:06, <feng...@intel.com> wrote: > >> -----Original Message----- >> From: xen-devel-boun...@lists.xen.org >> [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Andrew Cooper >> Sent: Thursday, June 25, 2015 2:35 AM >> To: Wu, Feng; xen-devel@lists.xen.org >> Cc: george.dun...@eu.citrix.com; Zhang, Yang Z; Tian, Kevin; k...@xen.org; >> jbeul...@suse.com >> 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...@intel.com> >> > --- >> > 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.
I'm not convinced such a #UD would be a significant problem: Looking at the disassembly will show the cause right away. The out of line ud2-s in some of VMX'es inline assembly wrappers are far worse. As to panic()ing from assembly code: movq $<string-label>, %rdi call panic >> 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))); How would that help? The table entries hardware uses are supposed to be 16-byte aligned anyway, aren't they? I think Andrew's "enforce" really means ASSERT() or BUG_ON(), again to avoid an unqualified exception. However - see above. Plus, all that said, without having seen the actual use sites of cmpxchg16b yet, I'm not at all convinced we really need this patch. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel