[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v2] x86/atomic: Improvements and simplifications to assembly constraints


  • To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Mon, 18 Mar 2019 11:27:21 +0000
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 18 Mar 2019 11:27:38 +0000
  • Ironport-data: A9a23:fo+Uqa0IswQulR9+GfbDix16xeBEqk9dZSB6s9h1AyB0oetYVcj0zc +WGTbVwNsgavuIvyxlfLitLZtsv2cZBEhfEVWGkinpbSBZNz7of+0rcHPUBKUW/Z9pK4uBVd oHniPfxqduBHWFVVjpE/IBGvblsexkYaPWdjfssPWRyUU2sGcc7Ike4OG0t91Ud4OKEh5w+5 qrUmqSkYDQNQs2ebcbxLOwB1OZ8y8zFGNFkWLLaHO2Z+SiHBjn9L8dNxRwR7TAKhCS3xt9i7 OBj2anUWKD3pl9m+EYNqkGCHdzrsGSlxOixlSXXFhjkl3ThwFI/XnrLcwB6wPq4RStmYHGW/ vX0946ZIV6ka6kpDCH90VqOiBOzWJJZ9miwlrX5GlvNghK9Po62JfNoTGCf4f5cb+LfqfdXU 6baOXPQLYtGzsNaFBzE3mbpL3F17gaK4pv9c3z1WYyCvkUUK+iEXZnEAMtYA6rhyQGSCMRMP mkyPhosStXb86dx5PEM78JwGEZkYDnB8XWKd1cyHFEzqYSwynjVQsyloZirkT3+qfLuJITaU qiUcaXT+Rn/CynAwB7oRne7C3B9p8Z47AQO0ckb2P1T2oMJijMT30xhdnRBhhE/6cHshwH2c QUEJM6R2cJhDMmDWbiMKKL+XyBTqShSFk0G6j/AC/CnV7X73WMccFCNf41pfJH6eCtKZOT8n myIKXgOYAE0Q/Lf+w3owtuzMP2XAWKSihaHEiV9uzqqXazVk7en0Z2sWsApSEeh+3jHwb6Hy W/MpvJUMPkbsuY4+Fc6paYjDBwaV9P3uqpZV4lzUiiDOjM7dF29ASGqspZClHO9deVLR2Xbv OaSVwK59A/Lz3FtiVgBbPNudDkunxLLtK8C0OJHmdGC0jiItmEYVV5mDpHoptZTSgKGhPKAs DMV7C07y755L9EH3IKDQFuy4zye0oaF92SM8U7fKZ3KexpbwuyyOjYWEI6tk/CWz2Q1jNGLk BCNX3aldDQzVHH4m6d5JP8lQDEbj/pjmQ7CvRjaEgvFMhgBWT9373QK4M5BVfarDh/kG5Cl0 mDg8KEC3hLf3K9TUyXySTMrNjRTWpzjVDnV59laMVDjW/67zzT9Z4/07WdlAR96TaSf6BQIc 1noOVK4TyzBRJ3UHC6tNsMUoPpJ3ac5OpQ+gqYs+4fURy2dOy4RGtt5XKFYPL95lCIMPS8vt FOzVMZweLskQZc5l0LhcZ1w+F0wPKy5hYn2B5+Wi+6CI7ImCXubr7wLaMEYuhjmBV3Ai6jeZ A4vdqf1pbIUPvqQ0IZThr1oIBYusEl8nCBayhQ0GtZjCrqalv8ulRl8XbjljU816FNh5hTKL 7BbIOSPmdizR/T5r/Ccb32IeQn1utOf99W124HhOG1EgxilfpGVewsxlAcnvDU3be1UEIKcp jXqjnTqLqj++qJ3LTCdr6ezV5RMghHqANNG2bnNUJutLdimfmUjSFRO0zkJz8v1IryLZLWxi AU3VvB8HH0Dyx322BRpGUzhWrJIXYFa95AN6lQ+cqWdMrleJBSEsU/KxHZq4SKvs/70bWwGE evW5RVebsW3ZqLpUgd9JsOcUz3vgs1rU0+yxIfRySdUXqrl6fPf4eK3qVLSTnL70TVww2Mdn z61Jl1nCHvk5XvsB2zFnBe1kpAouMYJqPBqoqHPMEVSL4S0ZmO8kqEOZepwZ4qP9kC1LCjy2 bqOQBm4LUbigEr9Q4dT4zroVchFeMLKXInUCDp23Vw5jwr38LjLX1R9AteOULLbbVjfCixUZ BpUJxNmvVjeu/wzg7WDly1dD80X23aHXh0yHHFspfC02UtCtkqrieFBQJg
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

 * Some of the single-byte versions specify "=q" as the output.  This is a
   remnent of the 32bit build and can be relaxed to "=r" in 64bit builds.
 * Constraints in the form "=r" (x) : "0" (x) can be folded to just "+r" (x)
 * Switch to using named parameters (mostly for legibility) which in
   particular helps with...
 * __xchg(), __cmpxchg() and __cmpxchg_user() modify their memory operand, so
   must list it as an output operand.  This only works because they each have
   a memory clobber to give the construct full compiler-barrier properties.
 * Every memory operand has an explicit known size.  Letting the compiler see
   the real size rather than obscuring it with __xg() allows for the removal
   of the instruction size suffixes without introducing ambiguity.
 * Misc style changes.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

v2:
 * Correct comment WRT 32bit builds and "=q" constraints
 * Drop semicolons after the lock prefix
 * Further reduce the use of positional parameters

The reason the volatile cast in __cmpxchg_user() can't be dropped is because
without it, the compiler uses a stack copy rather than the in-memory copy,
which ends up tripping:

  /* Allowed to change in Accessed/Dirty flags only. */
  BUG_ON((t ^ old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
---
 xen/include/asm-x86/system.h        | 99 +++++++++++++++++--------------------
 xen/include/asm-x86/x86_64/system.h | 38 +++++++-------
 2 files changed, 65 insertions(+), 72 deletions(-)

diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index 3246797..ba5a3b5 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -24,9 +24,6 @@ static inline void clflush(const void *p)
 #define xchg(ptr,v) \
     ((__typeof__(*(ptr)))__xchg((unsigned long)(v),(ptr),sizeof(*(ptr))))
 
-struct __xchg_dummy { unsigned long a[100]; };
-#define __xg(x) ((volatile struct __xchg_dummy *)(x))
-
 #include <asm/x86_64/system.h>
 
 /*
@@ -40,28 +37,24 @@ static always_inline unsigned long __xchg(
     switch ( size )
     {
     case 1:
-        asm volatile ( "xchgb %b0,%1"
-                       : "=q" (x)
-                       : "m" (*__xg(ptr)), "0" (x)
-                       : "memory" );
+        asm volatile ( "xchg %b[x], %[ptr]"
+                       : [x] "+r" (x), [ptr] "+m" (*(uint8_t *)ptr)
+                       :: "memory" );
         break;
     case 2:
-        asm volatile ( "xchgw %w0,%1"
-                       : "=r" (x)
-                       : "m" (*__xg(ptr)), "0" (x)
-                       : "memory" );
+        asm volatile ( "xchg %w[x], %[ptr]"
+                       : [x] "+r" (x), [ptr] "+m" (*(uint16_t *)ptr)
+                       :: "memory" );
         break;
     case 4:
-        asm volatile ( "xchgl %k0,%1"
-                       : "=r" (x)
-                       : "m" (*__xg(ptr)), "0" (x)
-                       : "memory" );
+        asm volatile ( "xchg %k[x], %[ptr]"
+                       : [x] "+r" (x), [ptr] "+m" (*(uint32_t *)ptr)
+                       :: "memory" );
         break;
     case 8:
-        asm volatile ( "xchgq %0,%1"
-                       : "=r" (x)
-                       : "m" (*__xg(ptr)), "0" (x)
-                       : "memory" );
+        asm volatile ( "xchg %q[x], %[ptr]"
+                       : [x] "+r" (x), [ptr] "+m" (*(uint64_t *)ptr)
+                       :: "memory" );
         break;
     }
     return x;
@@ -80,31 +73,27 @@ static always_inline unsigned long __cmpxchg(
     switch ( size )
     {
     case 1:
-        asm volatile ( "lock; cmpxchgb %b1,%2"
-                       : "=a" (prev)
-                       : "q" (new), "m" (*__xg(ptr)),
-                       "0" (old)
+        asm volatile ( "lock cmpxchg %b[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint8_t *)ptr)
+                       : [new] "r" (new), "a" (old)
                        : "memory" );
         return prev;
     case 2:
-        asm volatile ( "lock; cmpxchgw %w1,%2"
-                       : "=a" (prev)
-                       : "r" (new), "m" (*__xg(ptr)),
-                       "0" (old)
+        asm volatile ( "lock cmpxchg %w[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint16_t *)ptr)
+                       : [new] "r" (new), "a" (old)
                        : "memory" );
         return prev;
     case 4:
-        asm volatile ( "lock; cmpxchgl %k1,%2"
-                       : "=a" (prev)
-                       : "r" (new), "m" (*__xg(ptr)),
-                       "0" (old)
+        asm volatile ( "lock cmpxchg %k[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint32_t *)ptr)
+                       : [new] "r" (new), "a" (old)
                        : "memory" );
         return prev;
     case 8:
-        asm volatile ( "lock; cmpxchgq %1,%2"
-                       : "=a" (prev)
-                       : "r" (new), "m" (*__xg(ptr)),
-                       "0" (old)
+        asm volatile ( "lock cmpxchg %q[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint64_t *)ptr)
+                       : [new] "r" (new), "a" (old)
                        : "memory" );
         return prev;
     }
@@ -119,24 +108,24 @@ static always_inline unsigned long cmpxchg_local_(
     switch ( size )
     {
     case 1:
-        asm volatile ( "cmpxchgb %b2, %1"
-                       : "=a" (prev), "+m" (*(uint8_t *)ptr)
-                       : "q" (new), "0" (old) );
+        asm volatile ( "cmpxchg %b[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint8_t *)ptr)
+                       : [new] "r" (new), "a" (old) );
         break;
     case 2:
-        asm volatile ( "cmpxchgw %w2, %1"
-                       : "=a" (prev), "+m" (*(uint16_t *)ptr)
-                       : "r" (new), "0" (old) );
+        asm volatile ( "cmpxchg %w[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint16_t *)ptr)
+                       : [new] "r" (new), "a" (old) );
         break;
     case 4:
-        asm volatile ( "cmpxchgl %k2, %1"
-                       : "=a" (prev), "+m" (*(uint32_t *)ptr)
-                       : "r" (new), "0" (old) );
+        asm volatile ( "cmpxchg %k[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint32_t *)ptr)
+                       : [new] "r" (new), "a" (old) );
         break;
     case 8:
-        asm volatile ( "cmpxchgq %2, %1"
-                       : "=a" (prev), "+m" (*(uint64_t *)ptr)
-                       : "r" (new), "0" (old) );
+        asm volatile ( "cmpxchg %q[new], %[ptr]"
+                       : "=a" (prev), [ptr] "+m" (*(uint64_t *)ptr)
+                       : [new] "r" (new), "a" (old) );
         break;
     }
 
@@ -162,23 +151,23 @@ static always_inline unsigned long __xadd(
     switch ( size )
     {
     case 1:
-        asm volatile ( "lock; xaddb %b0,%1"
-                       : "+r" (v), "+m" (*__xg(ptr))
+        asm volatile ( "lock xadd %b[v], %[ptr]"
+                       : [v] "+r" (v), [ptr] "+m" (*(uint8_t *)ptr)
                        :: "memory");
         return v;
     case 2:
-        asm volatile ( "lock; xaddw %w0,%1"
-                       : "+r" (v), "+m" (*__xg(ptr))
+        asm volatile ( "lock xadd %w[v], %[ptr]"
+                       : [v] "+r" (v), [ptr] "+m" (*(uint16_t *)ptr)
                        :: "memory");
         return v;
     case 4:
-        asm volatile ( "lock; xaddl %k0,%1"
-                       : "+r" (v), "+m" (*__xg(ptr))
+        asm volatile ( "lock xadd %k[v], %[ptr]"
+                       : [v] "+r" (v), [ptr] "+m" (*(uint32_t *)ptr)
                        :: "memory");
         return v;
     case 8:
-        asm volatile ( "lock; xaddq %q0,%1"
-                       : "+r" (v), "+m" (*__xg(ptr))
+        asm volatile ( "lock xadd %q[v], %[ptr]"
+                       : [v] "+r" (v), [ptr] "+m" (*(uint64_t *)ptr)
                        :: "memory");
 
         return v;
diff --git a/xen/include/asm-x86/x86_64/system.h 
b/xen/include/asm-x86/x86_64/system.h
index fae57ba..5b6e964 100644
--- a/xen/include/asm-x86/x86_64/system.h
+++ b/xen/include/asm-x86/x86_64/system.h
@@ -24,9 +24,10 @@ static always_inline __uint128_t __cmpxchg16b(
     ASSERT(cpu_has_cx16);
 
     /* Don't use "=A" here - clang can't deal with that. */
-    asm volatile ( "lock; cmpxchg16b %2"
-                   : "=d" (prev.hi), "=a" (prev.lo), "+m" (*__xg(ptr))
-                   : "c" (new.hi), "b" (new.lo), "0" (old.hi), "1" (old.lo) );
+    asm volatile ( "lock cmpxchg16b %[ptr]"
+                   : "=d" (prev.hi), "=a" (prev.lo),
+                     [ptr] "+m" (*(__uint128_t *)ptr)
+                   : "c" (new.hi), "b" (new.lo), "d" (old.hi), "a" (old.lo) );
 
     return prev.raw;
 }
@@ -42,9 +43,10 @@ static always_inline __uint128_t cmpxchg16b_local_(
     ASSERT(cpu_has_cx16);
 
     /* Don't use "=A" here - clang can't deal with that. */
-    asm volatile ( "cmpxchg16b %2"
-                   : "=d" (prev.hi), "=a" (prev.lo), "+m" (*(__uint128_t *)ptr)
-                   : "c" (new.hi), "b" (new.lo), "0" (old.hi), "1" (old.lo) );
+    asm volatile ( "cmpxchg16b %[ptr]"
+                   : "=d" (prev.hi), "=a" (prev.lo),
+                     [ptr] "+m" (*(__uint128_t *)ptr)
+                   : "c" (new.hi), "b" (new.lo), "d" (old.hi), "a" (old.lo) );
 
     return prev.raw;
 }
@@ -63,36 +65,38 @@ static always_inline __uint128_t cmpxchg16b_local_(
  * If no fault occurs then _o is updated to the value we saw at _p. If this
  * is the same as the initial value of _o then _n is written to location _p.
  */
-#define __cmpxchg_user(_p,_o,_n,_isuff,_oppre,_regtype)                 \
+#define __cmpxchg_user(_p, _o, _n, _oppre)                              \
     stac();                                                             \
     asm volatile (                                                      \
-        "1: lock; cmpxchg"_isuff" %"_oppre"2,%3\n"                      \
+        "1: lock cmpxchg %"_oppre"[new], %[ptr]\n"                      \
         "2:\n"                                                          \
         ".section .fixup,\"ax\"\n"                                      \
-        "3:     movl $1,%1\n"                                           \
+        "3:     movl $1, %[rc]\n"                                       \
         "       jmp 2b\n"                                               \
         ".previous\n"                                                   \
         _ASM_EXTABLE(1b, 3b)                                            \
-        : "=a" (_o), "=r" (_rc)                                         \
-        : _regtype (_n), "m" (*__xg((volatile void *)_p)), "0" (_o), "1" (0) \
+        : "+a" (_o), [rc] "=r" (_rc),                                   \
+          [ptr] "+m" (*(volatile typeof(*(_p)) *)(_p))                  \
+        : [new] "r" (_n), "[rc]" (0)                                    \
         : "memory");                                                    \
     clac()
 
-#define cmpxchg_user(_p,_o,_n)                                          \
+#define cmpxchg_user(_p, _o, _n)                                        \
 ({                                                                      \
     int _rc;                                                            \
-    switch ( sizeof(*(_p)) ) {                                          \
+    switch ( sizeof(*(_p)) )                                            \
+    {                                                                   \
     case 1:                                                             \
-        __cmpxchg_user(_p,_o,_n,"b","b","q");                           \
+        __cmpxchg_user(_p, _o, _n, "b");                                \
         break;                                                          \
     case 2:                                                             \
-        __cmpxchg_user(_p,_o,_n,"w","w","r");                           \
+        __cmpxchg_user(_p, _o, _n, "w");                                \
         break;                                                          \
     case 4:                                                             \
-        __cmpxchg_user(_p,_o,_n,"l","k","r");                           \
+        __cmpxchg_user(_p, _o, _n, "k");                                \
         break;                                                          \
     case 8:                                                             \
-        __cmpxchg_user(_p,_o,_n,"q","","r");                            \
+        __cmpxchg_user(_p, _o, _n, "q");                                \
         break;                                                          \
     }                                                                   \
     _rc;                                                                \
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.