On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:

quoted

On Mon, Jul 25, 2022 at 10:10:09PM +0200, Pali Rohár wrote:

quoted

On Monday 25 July 2022 16:20:49 Christophe Leroy wrote:
Now I did again clean test with same Debian 10 cross compiler.

$ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git && cd linux
$ git checkout v5.15
$ make mpc85xx_smp_defconfig ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe-
$ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe-
$ cp -a vmlinux vmlinux.v5.15
$ git revert 9401f4e46cf6965e23738f70e149172344a01eef
$ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe-
$ cp -a vmlinux vmlinux.revert
$ powerpc-linux-gnuspe-objdump -d vmlinux.revert > vmlinux.revert.dump
$ powerpc-linux-gnuspe-objdump -d vmlinux.v5.15 > vmlinux.v5.15.dump
$ diff -Naurp vmlinux.v5.15.dump vmlinux.revert.dump

And there are:

-c000c304:      7d 20 f8 29     lwarx   r9,0,r31,1
+c000c304:      7d 20 f8 28     lwarx   r9,0,r31

I guess it must be reproducible this issue as I'm using regular
toolchain from distribution.

The kernel had

#define PPC_RAW_LWARX(t, a, b, eh)       (0x7c000028 | ___PPC_RT(t) | ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh))

and

#define PPC_LWARX(t, a, b, eh) stringify_in_c(.long PPC_RAW_LWARX(t, a, b, eh))

and

#ifdef CONFIG_PPC64
#define __PPC_EH(eh)    (((eh) & 0x1) << 0)
#else
#define __PPC_EH(eh)    0
#endif

but Christophe's 9401f4e46cf6 changed

-"1:    " PPC_LWARX(%0,0,%2,1) "\n\
+"1:    lwarx           %0,0,%2,1\n\

no longer checking CONFIG_PPC64.  That appears to be the bug.

Nice catch!

Now I have tried to apply following change on master (without reverting
anything)

diff --git a/arch/powerpc/include/asm/simple_spinlock.h b/arch/powerpc/include/asm/simple_spinlock.h
index 7ae6aeef8464..72d3657fd2f7 100644
--- a/arch/powerpc/include/asm/simple_spinlock.h
+++ b/arch/powerpc/include/asm/simple_spinlock.h

@@ -51,7 +51,7 @@ static inline unsigned long __arch_spin_trylock(arch_spinlock_t *lock)
  
  	token = LOCK_TOKEN;
  	__asm__ __volatile__(
-"1:	lwarx		%0,0,%2,1\n\
+"1:	lwarx		%0,0,%2,0\n\
  	cmpwi		0,%0,0\n\
  	bne-		2f\n\
  	stwcx.		%1,0,%2\n\

@@ -158,7 +158,7 @@ static inline long __arch_read_trylock(arch_rwlock_t *rw)
  	long tmp;
  
  	__asm__ __volatile__(
-"1:	lwarx		%0,0,%1,1\n"
+"1:	lwarx		%0,0,%1,0\n"
  	__DO_SIGN_EXTEND
  "	addic.		%0,%0,1\n\
  	ble-		2f\n"

@@ -182,7 +182,7 @@ static inline long __arch_write_trylock(arch_rwlock_t *rw)
  
  	token = WRLOCK_TOKEN;
  	__asm__ __volatile__(
-"1:	lwarx		%0,0,%2,1\n\
+"1:	lwarx		%0,0,%2,0\n\
  	cmpwi		0,%0,0\n\
  	bne-		2f\n"
  "	stwcx.		%1,0,%2\n\

and with this change, objdump showed exactly same result as if I revert
that problematic commit on top of master branch.

I guess that simple_spinlock.h should be fixed to pass 1 to lwarx for
CONFIG_PPC64 and 0 otherwise.

Christophe, are you going to look at it?