Re: x86-fix-cmpxchg.patch added to -mm tree

Re: x86-fix-cmpxchg.patch added to -mm tree

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 4374 bytes --]

>>> Dave Jones <davej@redhat.com> 10.09.05 02:30:38 >>>
>On Fri, Sep 09, 2005 at 05:06:56PM -0700, Andrew Morton wrote:
>
> > - cmpxchg8b gets disabled when the minimum specified hardware
architectur
> >   doesn't support it (like was already happening for the byte,
word, and
> >   long ones).
> >
> > +config X86_CMPXCHG64
> > +	bool
> > +	depends on !M386 && !M486 && !MCYRIXIII && !MGEODEGX1
> > +	default y
>
>This is wrong.  All the Winchip/CyrixIII CPUs do indeed have cx8.
>Though cpuid will report that it's missing until We explicitly enable
>it in init_c3().  Whilst it's "disabled" however, the instruction
does
>run perfectly fine.   This was done because Win NT wouldn't boot if
it
>found a non Intel CPU which had cx8 iirc.
>
>I'm not familiar with the Geode enough to answer definitively.
>Alan seems to be the Geode-guru, and may still have datasheets for
that.

Resubmitting adjusted patch (taking into account also Alan's later
response).

(Note: Patch also attached because the inline version is certain to
get
line wrapped.)

This adjusts i386's cmpxchg patterns so that
- for word and long cmpxchg-es the compiler can utilize all possible
  registers
- cmpxchg8b gets disabled when the minimum specified hardware
architectur
  doesn't support it (like was already happening for the byte, word,
and
  long ones).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru 2.6.13/arch/i386/Kconfig
2.6.13-i386-cmpxchg/arch/i386/Kconfig
--- 2.6.13/arch/i386/Kconfig	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-i386-cmpxchg/arch/i386/Kconfig	2005-09-01
11:45:29.000000000 +0200
@@ -412,6 +412,11 @@ config X86_POPAD_OK
 	depends on !M386
 	default y
 
+config X86_CMPXCHG64
+	bool
+	depends on !M386 && !M486
+	default y
+
 config X86_ALIGNMENT_16
 	bool
 	depends on MWINCHIP3D || MWINCHIP2 || MWINCHIPC6 || MCYRIXIII ||
X86_ELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 ||
MGEODEGX1
diff -Npru 2.6.13/include/asm-i386/system.h
2.6.13-i386-cmpxchg/include/asm-i386/system.h
--- 2.6.13/include/asm-i386/system.h	2005-08-29 01:41:01.000000000
+0200
+++ 2.6.13-i386-cmpxchg/include/asm-i386/system.h	2005-09-07
11:37:25.000000000 +0200
@@ -149,6 +149,8 @@ struct __xchg_dummy { unsigned long a[10
 #define __xg(x) ((struct __xchg_dummy *)(x))
 
 
+#ifdef CONFIG_X86_CMPXCHG64
+
 /*
  * The semantics of XCHGCMP8B are a bit strange, this is why
  * there is a loop and the loading of %%eax and %%edx has to
@@ -203,6 +205,8 @@ static inline void __set_64bit_var (unsi
  __set_64bit(ptr, (unsigned int)(value), (unsigned
int)((value)>>32ULL) ) : \
  __set_64bit(ptr, ll_low(value), ll_high(value)) )
 
+#endif
+
 /*
  * Note: no "lock" prefix even on SMP: xchg always implies lock
anyway
  * Note 2: xchg has side effect, so that attribute volatile is
necessary,
@@ -241,7 +245,6 @@ static inline unsigned long __xchg(unsig
 
 #ifdef CONFIG_X86_CMPXCHG
 #define __HAVE_ARCH_CMPXCHG 1
-#endif
 
 static inline unsigned long __cmpxchg(volatile void *ptr, unsigned
long old,
 				      unsigned long new, int size)
@@ -257,13 +260,13 @@ static inline unsigned long __cmpxchg(vo
 	case 2:
 		__asm__ __volatile__(LOCK_PREFIX "cmpxchgw %w1,%2"
 				     : "=a"(prev)
-				     : "q"(new), "m"(*__xg(ptr)),
"0"(old)
+				     : "r"(new), "m"(*__xg(ptr)),
"0"(old)
 				     : "memory");
 		return prev;
 	case 4:
 		__asm__ __volatile__(LOCK_PREFIX "cmpxchgl %1,%2"
 				     : "=a"(prev)
-				     : "q"(new), "m"(*__xg(ptr)),
"0"(old)
+				     : "r"(new), "m"(*__xg(ptr)),
"0"(old)
 				     : "memory");
 		return prev;
 	}
@@ -273,6 +276,30 @@ static inline unsigned long __cmpxchg(vo
 #define cmpxchg(ptr,o,n)\
 	((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)(o),\
 					(unsigned
long)(n),sizeof(*(ptr))))
+
+#endif
+
+#ifdef CONFIG_X86_CMPXCHG64
+
+static inline unsigned long long __cmpxchg64(volatile void *ptr,
unsigned long long old,
+				      unsigned long long new)
+{
+	unsigned long long prev;
+	__asm__ __volatile__(LOCK_PREFIX "cmpxchg8b %3"
+			     : "=A"(prev)
+			     : "b"((unsigned long)new),
+			       "c"((unsigned long)(new >> 32)),
+			       "m"(*__xg(ptr)),
+			       "0"(old)
+			     : "memory");
+	return prev;
+}
+
+#define cmpxchg64(ptr,o,n)\
+	((__typeof__(*(ptr)))__cmpxchg64((ptr),(unsigned long
long)(o),\
+					(unsigned long long)(n)))
+
+#endif
     
 #ifdef __KERNEL__
 struct alt_instr { 


[-- Attachment #2: linux-2.6.13-i386-cmpxchg.patch --]
[-- Type: application/octet-stream, Size: 3414 bytes --]

(Note: Patch also attached because the inline version is certain to get
line wrapped.)

This adjusts i386's cmpxchg patterns so that
- for word and long cmpxchg-es the compiler can utilize all possible
  registers
- cmpxchg8b gets disabled when the minimum specified hardware architectur
  doesn't support it (like was already happening for the byte, word, and
  long ones).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru 2.6.13/arch/i386/Kconfig 2.6.13-i386-cmpxchg/arch/i386/Kconfig
--- 2.6.13/arch/i386/Kconfig	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-i386-cmpxchg/arch/i386/Kconfig	2005-09-01 11:45:29.000000000 +0200
@@ -412,6 +412,11 @@ config X86_POPAD_OK
 	depends on !M386
 	default y
 
+config X86_CMPXCHG64
+	bool
+	depends on !M386 && !M486
+	default y
+
 config X86_ALIGNMENT_16
 	bool
 	depends on MWINCHIP3D || MWINCHIP2 || MWINCHIPC6 || MCYRIXIII || X86_ELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
diff -Npru 2.6.13/include/asm-i386/system.h 2.6.13-i386-cmpxchg/include/asm-i386/system.h
--- 2.6.13/include/asm-i386/system.h	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-i386-cmpxchg/include/asm-i386/system.h	2005-09-07 11:37:25.000000000 +0200
@@ -149,6 +149,8 @@ struct __xchg_dummy { unsigned long a[10
 #define __xg(x) ((struct __xchg_dummy *)(x))
 
 
+#ifdef CONFIG_X86_CMPXCHG64
+
 /*
  * The semantics of XCHGCMP8B are a bit strange, this is why
  * there is a loop and the loading of %%eax and %%edx has to
@@ -203,6 +205,8 @@ static inline void __set_64bit_var (unsi
  __set_64bit(ptr, (unsigned int)(value), (unsigned int)((value)>>32ULL) ) : \
  __set_64bit(ptr, ll_low(value), ll_high(value)) )
 
+#endif
+
 /*
  * Note: no "lock" prefix even on SMP: xchg always implies lock anyway
  * Note 2: xchg has side effect, so that attribute volatile is necessary,
@@ -241,7 +245,6 @@ static inline unsigned long __xchg(unsig
 
 #ifdef CONFIG_X86_CMPXCHG
 #define __HAVE_ARCH_CMPXCHG 1
-#endif
 
 static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
 				      unsigned long new, int size)
@@ -257,13 +260,13 @@ static inline unsigned long __cmpxchg(vo
 	case 2:
 		__asm__ __volatile__(LOCK_PREFIX "cmpxchgw %w1,%2"
 				     : "=a"(prev)
-				     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+				     : "r"(new), "m"(*__xg(ptr)), "0"(old)
 				     : "memory");
 		return prev;
 	case 4:
 		__asm__ __volatile__(LOCK_PREFIX "cmpxchgl %1,%2"
 				     : "=a"(prev)
-				     : "q"(new), "m"(*__xg(ptr)), "0"(old)
+				     : "r"(new), "m"(*__xg(ptr)), "0"(old)
 				     : "memory");
 		return prev;
 	}
@@ -273,6 +276,30 @@ static inline unsigned long __cmpxchg(vo
 #define cmpxchg(ptr,o,n)\
 	((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)(o),\
 					(unsigned long)(n),sizeof(*(ptr))))
+
+#endif
+
+#ifdef CONFIG_X86_CMPXCHG64
+
+static inline unsigned long long __cmpxchg64(volatile void *ptr, unsigned long long old,
+				      unsigned long long new)
+{
+	unsigned long long prev;
+	__asm__ __volatile__(LOCK_PREFIX "cmpxchg8b %3"
+			     : "=A"(prev)
+			     : "b"((unsigned long)new),
+			       "c"((unsigned long)(new >> 32)),
+			       "m"(*__xg(ptr)),
+			       "0"(old)
+			     : "memory");
+	return prev;
+}
+
+#define cmpxchg64(ptr,o,n)\
+	((__typeof__(*(ptr)))__cmpxchg64((ptr),(unsigned long long)(o),\
+					(unsigned long long)(n)))
+
+#endif
     
 #ifdef __KERNEL__
 struct alt_instr { 

Re: x86-fix-cmpxchg.patch added to -mm tree

[Denis Vlasenko]

On Monday 12 September 2005 09:41, Jan Beulich wrote:
> >>> Dave Jones <davej@redhat.com> 10.09.05 02:30:38 >>>
> >On Fri, Sep 09, 2005 at 05:06:56PM -0700, Andrew Morton wrote:
> >
> > > - cmpxchg8b gets disabled when the minimum specified hardware
> architectur
> > >   doesn't support it (like was already happening for the byte,
> word, and
> > >   long ones).
> > >
> > > +config X86_CMPXCHG64
> > > +	bool
> > > +	depends on !M386 && !M486 && !MCYRIXIII && !MGEODEGX1
> > > +	default y
> >
> >This is wrong.  All the Winchip/CyrixIII CPUs do indeed have cx8.
> >Though cpuid will report that it's missing until We explicitly enable
> >it in init_c3().  Whilst it's "disabled" however, the instruction does
> >run perfectly fine.   This was done because Win NT wouldn't boot if it
> >found a non Intel CPU which had cx8 iirc.

Can this be added to comments in the right place?

> >I'm not familiar with the Geode enough to answer definitively.
> >Alan seems to be the Geode-guru, and may still have datasheets for
> that.
> 
> Resubmitting adjusted patch (taking into account also Alan's later
> response).
--
vda