x86, asm: Clean up and simplify set_64bit()

Clean up and simplify set_64bit().  This code is quite old (1.3.11)
and contains a fair bit of auxilliary machinery that current versions
of gcc handle just fine automatically.  Worse, the auxilliary
machinery can actually cause an unnecessary spill to memory.

Furthermore, the loading of the old value inside the loop in the
32-bit case is unnecessary: if the value doesn't match, the CMPXCHG8B
instruction will already have loaded the "new previous" value for us.

Clean up the comment, too, and remove page references to obsolete
versions of the Intel SDM.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
LKML-Reference: <tip-*@vger.kernel.org>

arch/x86/include/asm/cmpxchg_32.h

@@ -53,60 +53,33 @@ struct __xchg_dummy {
 	__xchg((v), (ptr), sizeof(*ptr))
 
 /*
- * The semantics of XCHGCMP8B are a bit strange, this is why
+ * CMPXCHG8B only writes to the target if we had the previous
- * there is a loop and the loading of %%eax and %%edx has to
+ * value in registers, otherwise it acts as a read and gives us the
- * be inside. This inlines well in most cases, the cached
+ * "new previous" value.  That is why there is a loop.  Preloading
- * cost is around ~38 cycles. (in the future we might want
+ * EDX:EAX is a performance optimization: in the common case it means
- * to do an SIMD/3DNOW!/MMX/FPU 64-bit store here, but that
+ * we need only one locked operation.
- * might have an implicit FPU-save as a cost, so it's not
- * clear which path to go.)
  *
- * cmpxchg8b must be used with the lock prefix here to allow
+ * A SIMD/3DNOW!/MMX/FPU 64-bit store here would require at the very
- * the instruction to be executed atomically, see page 3-102
+ * least an FPU save and/or %cr0.ts manipulation.
- * of the instruction set reference 24319102.pdf. We need
+ *
- * the reader side to see the coherent 64bit value.
+ * cmpxchg8b must be used with the lock prefix here to allow the
+ * instruction to be executed atomically.  We need to have the reader
+ * side to see the coherent 64bit value.
  */
-static inline void __set_64bit(unsigned long long *ptr,
+static inline void set_64bit(volatile u64 *ptr, u64 value)
-			       unsigned int low, unsigned int high)
 {
+	u32 low  = value;
+	u32 high = value >> 32;
+	u64 prev = *ptr;
+
 	asm volatile("\n1:\t"
-		     "movl (%1), %%eax\n\t"
+		     LOCK_PREFIX "cmpxchg8b %0\n\t"
-		     "movl 4(%1), %%edx\n\t"
-		     LOCK_PREFIX "cmpxchg8b (%1)\n\t"
 		     "jnz 1b"
-		     : "=m" (*ptr)
+		     : "=m" (*ptr), "+A" (prev)
-		     : "D" (ptr),
+		     : "b" (low), "c" (high)
-		       "b" (low),
+		     : "memory");
-		       "c" (high)
-		     : "ax", "dx", "memory");
 }
 
-static inline void __set_64bit_constant(unsigned long long *ptr,
-					unsigned long long value)
-{
-	__set_64bit(ptr, (unsigned int)value, (unsigned int)(value >> 32));
-}
-
-#define ll_low(x)	*(((unsigned int *)&(x)) + 0)
-#define ll_high(x)	*(((unsigned int *)&(x)) + 1)
-
-static inline void __set_64bit_var(unsigned long long *ptr,
-				   unsigned long long value)
-{
-	__set_64bit(ptr, ll_low(value), ll_high(value));
-}
-
-#define set_64bit(ptr, value)			\
-	(__builtin_constant_p((value))		\
-	 ? __set_64bit_constant((ptr), (value))	\
-	 : __set_64bit_var((ptr), (value)))
-
-#define _set_64bit(ptr, value)						\
-	(__builtin_constant_p(value)					\
-	 ? __set_64bit(ptr, (unsigned int)(value),			\
-		       (unsigned int)((value) >> 32))			\
-	 : __set_64bit(ptr, ll_low((value)), ll_high((value))))
-
 extern void __cmpxchg_wrong_size(void);
 
 /*

arch/x86/include/asm/cmpxchg_64.h

@@ -5,13 +5,11 @@
 
 #define __xg(x) ((volatile long *)(x))
 
-static inline void set_64bit(volatile unsigned long *ptr, unsigned long val)
+static inline void set_64bit(volatile u64 *ptr, u64 val)
 {
 	*ptr = val;
 }
 
-#define _set_64bit set_64bit
-
 extern void __xchg_wrong_size(void);
 extern void __cmpxchg_wrong_size(void);