Le 26/07/2022 à 10:34, Pali Rohár a écrit : > On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote: >> On Mon, Jul 25, 2022 at 10:10:09PM +0200, Pali Rohár wrote: >>> 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? >
Yes I will, but next week at the earliest. Christophe