linux-kernel - Re: Regression: Linux v5.15+ does not boot on Freescale P2020

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4933ed23-9d0e-cffc-7bc6-05426414894c@csgroup.eu>
Date:   Tue, 26 Jul 2022 14:30:49 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Pali Rohár <pali@...nel.org>,
        Segher Boessenkool <segher@...nel.crashing.org>
CC:     "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Regression: Linux v5.15+ does not boot on Freescale P2020



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