Hello!
Trying to boot mainline Linux kernel v5.15+, including current version
from master branch, on Freescale P2020 does not work. Kernel does not
print anything to serial console, seems that it does not work and after
timeout watchdog reset the board.
I run git bisect and it found following commit:
9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
commit 9401f4e46cf6965e23738f70e149172344a01eef
Author: Christophe Leroy <[email protected]>
Date: Tue Mar 2 08:48:11 2021 +0000
powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
Force the eh flag at 0 on PPC32.
Signed-off-by: Christophe Leroy <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
:040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
If I revert this commit then kernel boots correctly. It also boots fine
if I revert this commit on top of master branch.
Freescale P2020 has two 32-bit e500 powerpc cores.
Any idea why above commit is causing crash of the kernel? And why it is
needed? Could eh flag set to 0 cause deadlock?
I have looked into e500 Reference Manual for lwarx instruction (page 562)
https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf and
both 0 and 1 values for EH flag should be supported.
Hello,
Le 22/07/2022 à 11:09, Pali Rohár a écrit :
> Hello!
>
> Trying to boot mainline Linux kernel v5.15+, including current version
> from master branch, on Freescale P2020 does not work. Kernel does not
> print anything to serial console, seems that it does not work and after
> timeout watchdog reset the board.
Can you provide more information ? Which defconfig or .config, which
version of gcc, etc ... ?
>
> I run git bisect and it found following commit:
>
> 9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
> commit 9401f4e46cf6965e23738f70e149172344a01eef
> Author: Christophe Leroy <[email protected]>
> Date: Tue Mar 2 08:48:11 2021 +0000
>
> powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
>
> Force the eh flag at 0 on PPC32.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> Signed-off-by: Michael Ellerman <[email protected]>
> Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
>
> :040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
>
>
> If I revert this commit then kernel boots correctly. It also boots fine
> if I revert this commit on top of master branch.
>
> Freescale P2020 has two 32-bit e500 powerpc cores.
>
> Any idea why above commit is causing crash of the kernel? And why it is
> needed? Could eh flag set to 0 cause deadlock?
Setting the eh flag to 0 is not supposed to be a change introduced by
that commit. Indeed that commit is not supposed to change anything at
all in the generated code.
Christophe
>
> I have looked into e500 Reference Manual for lwarx instruction (page 562)
> https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf and
> both 0 and 1 values for EH flag should be supported.
Hello,
On Saturday 23 July 2022 14:42:22 Christophe Leroy wrote:
> Hello,
>
> Le 22/07/2022 à 11:09, Pali Rohár a écrit :
> > Hello!
> >
> > Trying to boot mainline Linux kernel v5.15+, including current version
> > from master branch, on Freescale P2020 does not work. Kernel does not
> > print anything to serial console, seems that it does not work and after
> > timeout watchdog reset the board.
>
> Can you provide more information ? Which defconfig or .config, which
> version of gcc, etc ... ?
I used default defconfig for mpc85xx with gcc 8, compilation for e500
cores.
If you need exact .config content I can send it during week.
> >
> > I run git bisect and it found following commit:
> >
> > 9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
> > commit 9401f4e46cf6965e23738f70e149172344a01eef
> > Author: Christophe Leroy <[email protected]>
> > Date: Tue Mar 2 08:48:11 2021 +0000
> >
> > powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
> >
> > Force the eh flag at 0 on PPC32.
> >
> > Signed-off-by: Christophe Leroy <[email protected]>
> > Signed-off-by: Michael Ellerman <[email protected]>
> > Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
> >
> > :040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
> >
> >
> > If I revert this commit then kernel boots correctly. It also boots fine
> > if I revert this commit on top of master branch.
> >
> > Freescale P2020 has two 32-bit e500 powerpc cores.
> >
> > Any idea why above commit is causing crash of the kernel? And why it is
> > needed? Could eh flag set to 0 cause deadlock?
>
> Setting the eh flag to 0 is not supposed to be a change introduced by
> that commit. Indeed that commit is not supposed to change anything at
> all in the generated code.
My understanding of that commit is that it changed eh flag parameter
from 1 to 0 for 32-bit powerpc, including also p2020.
> Christophe
>
> >
> > I have looked into e500 Reference Manual for lwarx instruction (page 562)
> > https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf and
> > both 0 and 1 values for EH flag should be supported.
Pali Rohár <[email protected]> writes:
> On Saturday 23 July 2022 14:42:22 Christophe Leroy wrote:
>> Le 22/07/2022 à 11:09, Pali Rohár a écrit :
>> > Trying to boot mainline Linux kernel v5.15+, including current version
>> > from master branch, on Freescale P2020 does not work. Kernel does not
>> > print anything to serial console, seems that it does not work and after
>> > timeout watchdog reset the board.
>>
>> Can you provide more information ? Which defconfig or .config, which
>> version of gcc, etc ... ?
>
> I used default defconfig for mpc85xx with gcc 8, compilation for e500
> cores.
>
> If you need exact .config content I can send it during week.
>
>> > I run git bisect and it found following commit:
>> >
>> > 9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
>> > commit 9401f4e46cf6965e23738f70e149172344a01eef
>> > Author: Christophe Leroy <[email protected]>
>> > Date: Tue Mar 2 08:48:11 2021 +0000
>> >
>> > powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
>> >
>> > Force the eh flag at 0 on PPC32.
>> >
>> > Signed-off-by: Christophe Leroy <[email protected]>
>> > Signed-off-by: Michael Ellerman <[email protected]>
>> > Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
>> >
>> > :040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
>> >
>> >
>> > If I revert this commit then kernel boots correctly. It also boots fine
>> > if I revert this commit on top of master branch.
>> >
>> > Freescale P2020 has two 32-bit e500 powerpc cores.
>> >
>> > Any idea why above commit is causing crash of the kernel? And why it is
>> > needed? Could eh flag set to 0 cause deadlock?
>>
>> Setting the eh flag to 0 is not supposed to be a change introduced by
>> that commit. Indeed that commit is not supposed to change anything at
>> all in the generated code.
>
> My understanding of that commit is that it changed eh flag parameter
> from 1 to 0 for 32-bit powerpc, including also p2020.
Can you compare the disassembly before and after and find a place where
an instruction has changed?
cheers
On Monday 25 July 2022 18:20:01 Michael Ellerman wrote:
> Pali Rohár <[email protected]> writes:
> > On Saturday 23 July 2022 14:42:22 Christophe Leroy wrote:
> >> Le 22/07/2022 à 11:09, Pali Rohár a écrit :
> >> > Trying to boot mainline Linux kernel v5.15+, including current version
> >> > from master branch, on Freescale P2020 does not work. Kernel does not
> >> > print anything to serial console, seems that it does not work and after
> >> > timeout watchdog reset the board.
> >>
> >> Can you provide more information ? Which defconfig or .config, which
> >> version of gcc, etc ... ?
> >
> > I used default defconfig for mpc85xx with gcc 8, compilation for e500
> > cores.
> >
> > If you need exact .config content I can send it during week.
> >
> >> > I run git bisect and it found following commit:
> >> >
> >> > 9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
> >> > commit 9401f4e46cf6965e23738f70e149172344a01eef
> >> > Author: Christophe Leroy <[email protected]>
> >> > Date: Tue Mar 2 08:48:11 2021 +0000
> >> >
> >> > powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
> >> >
> >> > Force the eh flag at 0 on PPC32.
> >> >
> >> > Signed-off-by: Christophe Leroy <[email protected]>
> >> > Signed-off-by: Michael Ellerman <[email protected]>
> >> > Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
> >> >
> >> > :040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
> >> >
> >> >
> >> > If I revert this commit then kernel boots correctly. It also boots fine
> >> > if I revert this commit on top of master branch.
> >> >
> >> > Freescale P2020 has two 32-bit e500 powerpc cores.
> >> >
> >> > Any idea why above commit is causing crash of the kernel? And why it is
> >> > needed? Could eh flag set to 0 cause deadlock?
> >>
> >> Setting the eh flag to 0 is not supposed to be a change introduced by
> >> that commit. Indeed that commit is not supposed to change anything at
> >> all in the generated code.
> >
> > My understanding of that commit is that it changed eh flag parameter
> > from 1 to 0 for 32-bit powerpc, including also p2020.
>
> Can you compare the disassembly before and after and find a place where
> an instruction has changed?
>
> cheers
Yes, of course. Here is diff between output from objdump -d vmlinux.
original version --- is from git master branch and modified version +++
is the original version with reverted above problematic commit.
So the +++ version is the one which is working.
--- vmlinux.master.dump 2022-07-25 14:43:45.922239496 +0200
+++ vmlinux.revert.dump 2022-07-25 14:43:49.238259296 +0200
@@ -1,5 +1,5 @@
-vmlinux.master: file format elf32-powerpc
+vmlinux.revert: file format elf32-powerpc
Disassembly of section .head.text:
@@ -11213,7 +11213,7 @@ c000b850: 3f a0 c1 0f lis r29,-1611
c000b854: 81 02 00 04 lwz r8,4(r2)
c000b858: 3b fd 10 68 addi r31,r29,4200
c000b85c: 39 40 00 01 li r10,1
-c000b860: 7d 20 f8 29 lwarx r9,0,r31,1
+c000b860: 7d 20 f8 28 lwarx r9,0,r31
c000b864: 2c 09 00 00 cmpwi r9,0
c000b868: 40 82 00 10 bne c000b878 <die+0x68>
c000b86c: 7d 40 f9 2d stwcx. r10,0,r31
@@ -11227,7 +11227,7 @@ c000b888: 81 3e 00 1c lwz r9,28(r30
c000b88c: 7f 88 48 00 cmpw cr7,r8,r9
c000b890: 41 9e 00 38 beq cr7,c000b8c8 <die+0xb8>
c000b894: 39 40 00 01 li r10,1
-c000b898: 7d 20 f8 29 lwarx r9,0,r31,1
+c000b898: 7d 20 f8 28 lwarx r9,0,r31
c000b89c: 2c 09 00 00 cmpwi r9,0
c000b8a0: 40 82 00 10 bne c000b8b0 <die+0xa0>
c000b8a4: 7d 40 f9 2d stwcx. r10,0,r31
@@ -186495,7 +186495,7 @@ c00b173c: 3b 40 00 00 li r26,0
c00b1740: 3a e0 00 00 li r23,0
c00b1744: 7e c0 00 a6 mfmsr r22
c00b1748: 7c 00 01 46 wrteei 0
-c00b174c: 7f a0 c0 29 lwarx r29,0,r24,1
+c00b174c: 7f a0 c0 28 lwarx r29,0,r24
c00b1750: 2c 1d 00 00 cmpwi r29,0
c00b1754: 40 82 00 10 bne c00b1764 <rcu_gp_init+0x15c>
c00b1758: 7f 20 c1 2d stwcx. r25,0,r24
@@ -187821,7 +187821,7 @@ c00b2b7c: 3f e0 c1 0b lis r31,-1611
c00b2b80: 38 c0 00 01 li r6,1
c00b2b84: 3b ff c5 20 addi r31,r31,-15072
c00b2b88: 38 ff 02 20 addi r7,r31,544
-c00b2b8c: 7d 00 38 29 lwarx r8,0,r7,1
+c00b2b8c: 7d 00 38 28 lwarx r8,0,r7
c00b2b90: 2c 08 00 00 cmpwi r8,0
c00b2b94: 40 82 00 10 bne c00b2ba4 <rcu_cpu_starting+0xc0>
c00b2b98: 7c c0 39 2d stwcx. r6,0,r7
@@ -187947,7 +187947,7 @@ c00b2d6c: 3f a0 c1 0b lis r29,-1611
c00b2d70: 39 00 00 01 li r8,1
c00b2d74: 3b bd c5 20 addi r29,r29,-15072
c00b2d78: 39 3d 02 20 addi r9,r29,544
-c00b2d7c: 7d 40 48 29 lwarx r10,0,r9,1
+c00b2d7c: 7d 40 48 28 lwarx r10,0,r9
c00b2d80: 2c 0a 00 00 cmpwi r10,0
c00b2d84: 40 82 00 10 bne c00b2d94 <rcu_report_dead+0x90>
c00b2d88: 7d 00 49 2d stwcx. r8,0,r9
@@ -2527838,7 +2527838,7 @@ c096b0e4: 4e 80 00 20 blr
c096b0e8 <_raw_spin_trylock>:
c096b0e8: 94 21 ff f0 stwu r1,-16(r1)
c096b0ec: 39 40 00 01 li r10,1
-c096b0f0: 7d 20 18 29 lwarx r9,0,r3,1
+c096b0f0: 7d 20 18 28 lwarx r9,0,r3
c096b0f4: 2c 09 00 00 cmpwi r9,0
c096b0f8: 40 82 00 10 bne c096b108 <_raw_spin_trylock+0x20>
c096b0fc: 7d 40 19 2d stwcx. r10,0,r3
@@ -2527853,7 +2527853,7 @@ c096b11c: 4e 80 00 20 blr
c096b120 <_raw_read_trylock>:
c096b120: 94 21 ff f0 stwu r1,-16(r1)
-c096b124: 7d 20 18 29 lwarx r9,0,r3,1
+c096b124: 7d 20 18 28 lwarx r9,0,r3
c096b128: 35 29 00 01 addic. r9,r9,1
c096b12c: 40 81 00 10 ble c096b13c <_raw_read_trylock+0x1c>
c096b130: 7d 20 19 2d stwcx. r9,0,r3
@@ -2527869,7 +2527869,7 @@ c096b150: 4e 80 00 20 blr
c096b154 <_raw_write_trylock>:
c096b154: 94 21 ff f0 stwu r1,-16(r1)
c096b158: 39 40 ff ff li r10,-1
-c096b15c: 7d 20 18 29 lwarx r9,0,r3,1
+c096b15c: 7d 20 18 28 lwarx r9,0,r3
c096b160: 2c 09 00 00 cmpwi r9,0
c096b164: 40 82 00 10 bne c096b174 <_raw_write_trylock+0x20>
c096b168: 7d 40 19 2d stwcx. r10,0,r3
@@ -2527888,7 +2527888,7 @@ c096b190: 81 22 00 00 lwz r9,0(r2)
c096b194: 39 29 02 00 addi r9,r9,512
c096b198: 91 22 00 00 stw r9,0(r2)
c096b19c: 39 40 00 01 li r10,1
-c096b1a0: 7d 20 18 29 lwarx r9,0,r3,1
+c096b1a0: 7d 20 18 28 lwarx r9,0,r3
c096b1a4: 2c 09 00 00 cmpwi r9,0
c096b1a8: 40 82 00 10 bne c096b1b8 <_raw_spin_lock_bh+0x2c>
c096b1ac: 7d 40 19 2d stwcx. r10,0,r3
@@ -2527940,7 +2527940,7 @@ c096b240: 81 22 00 00 lwz r9,0(r2)
c096b244: 39 29 02 00 addi r9,r9,512
c096b248: 91 22 00 00 stw r9,0(r2)
c096b24c: 39 40 00 01 li r10,1
-c096b250: 7d 20 18 29 lwarx r9,0,r3,1
+c096b250: 7d 20 18 28 lwarx r9,0,r3
c096b254: 2c 09 00 00 cmpwi r9,0
c096b258: 40 82 00 10 bne c096b268 <_raw_spin_trylock_bh+0x34>
c096b25c: 7d 40 19 2d stwcx. r10,0,r3
@@ -2527997,7 +2527997,7 @@ c096b304: 94 21 ff f0 stwu r1,-16(r1
c096b308: 81 22 00 00 lwz r9,0(r2)
c096b30c: 39 29 02 00 addi r9,r9,512
c096b310: 91 22 00 00 stw r9,0(r2)
-c096b314: 7d 20 18 29 lwarx r9,0,r3,1
+c096b314: 7d 20 18 28 lwarx r9,0,r3
c096b318: 35 29 00 01 addic. r9,r9,1
c096b31c: 40 81 00 10 ble c096b32c <_raw_read_lock_bh+0x28>
c096b320: 7d 20 19 2d stwcx. r9,0,r3
@@ -2528018,7 +2528018,7 @@ c096b350: 81 22 00 00 lwz r9,0(r2)
c096b354: 39 29 02 00 addi r9,r9,512
c096b358: 91 22 00 00 stw r9,0(r2)
c096b35c: 39 40 ff ff li r10,-1
-c096b360: 7d 20 18 29 lwarx r9,0,r3,1
+c096b360: 7d 20 18 28 lwarx r9,0,r3
c096b364: 2c 09 00 00 cmpwi r9,0
c096b368: 40 82 00 10 bne c096b378 <_raw_write_lock_bh+0x2c>
c096b36c: 7d 40 19 2d stwcx. r10,0,r3
@@ -2528035,7 +2528035,7 @@ c096b394: 4b ff ff f4 b c096b388
c096b398 <_raw_read_lock>:
c096b398: 94 21 ff f0 stwu r1,-16(r1)
-c096b39c: 7d 20 18 29 lwarx r9,0,r3,1
+c096b39c: 7d 20 18 28 lwarx r9,0,r3
c096b3a0: 35 29 00 01 addic. r9,r9,1
c096b3a4: 40 81 00 10 ble c096b3b4 <_raw_read_lock+0x1c>
c096b3a8: 7d 20 19 2d stwcx. r9,0,r3
@@ -2528053,7 +2528053,7 @@ c096b3d0: 4b ff ff f4 b c096b3c4
c096b3d4 <_raw_read_lock_irq>:
c096b3d4: 94 21 ff f0 stwu r1,-16(r1)
c096b3d8: 7c 00 01 46 wrteei 0
-c096b3dc: 7d 20 18 29 lwarx r9,0,r3,1
+c096b3dc: 7d 20 18 28 lwarx r9,0,r3
c096b3e0: 35 29 00 01 addic. r9,r9,1
c096b3e4: 40 81 00 10 ble c096b3f4 <_raw_read_lock_irq+0x20>
c096b3e8: 7d 20 19 2d stwcx. r9,0,r3
@@ -2528073,7 +2528073,7 @@ c096b414: 94 21 ff f0 stwu r1,-16(r1
c096b418: 7c 6a 1b 78 mr r10,r3
c096b41c: 7c 60 00 a6 mfmsr r3
c096b420: 7c 00 01 46 wrteei 0
-c096b424: 7d 20 50 29 lwarx r9,0,r10,1
+c096b424: 7d 20 50 28 lwarx r9,0,r10
c096b428: 35 29 00 01 addic. r9,r9,1
c096b42c: 40 81 00 10 ble c096b43c <_raw_read_lock_irqsave+0x28>
c096b430: 7d 20 51 2d stwcx. r9,0,r10
@@ -2528091,7 +2528091,7 @@ c096b458: 4b ff ff f4 b c096b44c
c096b45c <_raw_spin_lock>:
c096b45c: 94 21 ff f0 stwu r1,-16(r1)
c096b460: 39 40 00 01 li r10,1
-c096b464: 7d 20 18 29 lwarx r9,0,r3,1
+c096b464: 7d 20 18 28 lwarx r9,0,r3
c096b468: 2c 09 00 00 cmpwi r9,0
c096b46c: 40 82 00 10 bne c096b47c <_raw_spin_lock+0x20>
c096b470: 7d 40 19 2d stwcx. r10,0,r3
@@ -2528110,7 +2528110,7 @@ c096b49c <_raw_spin_lock_irq>:
c096b49c: 94 21 ff f0 stwu r1,-16(r1)
c096b4a0: 7c 00 01 46 wrteei 0
c096b4a4: 39 40 00 01 li r10,1
-c096b4a8: 7d 20 18 29 lwarx r9,0,r3,1
+c096b4a8: 7d 20 18 28 lwarx r9,0,r3
c096b4ac: 2c 09 00 00 cmpwi r9,0
c096b4b0: 40 82 00 10 bne c096b4c0 <_raw_spin_lock_irq+0x24>
c096b4b4: 7d 40 19 2d stwcx. r10,0,r3
@@ -2528131,7 +2528131,7 @@ c096b4e4: 7c 6a 1b 78 mr r10,r3
c096b4e8: 7c 60 00 a6 mfmsr r3
c096b4ec: 7c 00 01 46 wrteei 0
c096b4f0: 39 00 00 01 li r8,1
-c096b4f4: 7d 20 50 29 lwarx r9,0,r10,1
+c096b4f4: 7d 20 50 28 lwarx r9,0,r10
c096b4f8: 2c 09 00 00 cmpwi r9,0
c096b4fc: 40 82 00 10 bne c096b50c <_raw_spin_lock_irqsave+0x2c>
c096b500: 7d 00 51 2d stwcx. r8,0,r10
@@ -2528149,7 +2528149,7 @@ c096b528: 4b ff ff f4 b c096b51c
c096b52c <_raw_write_lock_nested>:
c096b52c: 94 21 ff f0 stwu r1,-16(r1)
c096b530: 39 40 ff ff li r10,-1
-c096b534: 7d 20 18 29 lwarx r9,0,r3,1
+c096b534: 7d 20 18 28 lwarx r9,0,r3
c096b538: 2c 09 00 00 cmpwi r9,0
c096b53c: 40 82 00 10 bne c096b54c <_raw_write_lock_nested+0x20>
c096b540: 7d 40 19 2d stwcx. r10,0,r3
@@ -2528167,7 +2528167,7 @@ c096b568: 4b ff ff f4 b c096b55c
c096b56c <_raw_write_lock>:
c096b56c: 94 21 ff f0 stwu r1,-16(r1)
c096b570: 39 40 ff ff li r10,-1
-c096b574: 7d 20 18 29 lwarx r9,0,r3,1
+c096b574: 7d 20 18 28 lwarx r9,0,r3
c096b578: 2c 09 00 00 cmpwi r9,0
c096b57c: 40 82 00 10 bne c096b58c <_raw_write_lock+0x20>
c096b580: 7d 40 19 2d stwcx. r10,0,r3
@@ -2528186,7 +2528186,7 @@ c096b5ac <_raw_write_lock_irq>:
c096b5ac: 94 21 ff f0 stwu r1,-16(r1)
c096b5b0: 7c 00 01 46 wrteei 0
c096b5b4: 39 40 ff ff li r10,-1
-c096b5b8: 7d 20 18 29 lwarx r9,0,r3,1
+c096b5b8: 7d 20 18 28 lwarx r9,0,r3
c096b5bc: 2c 09 00 00 cmpwi r9,0
c096b5c0: 40 82 00 10 bne c096b5d0 <_raw_write_lock_irq+0x24>
c096b5c4: 7d 40 19 2d stwcx. r10,0,r3
@@ -2528207,7 +2528207,7 @@ c096b5f4: 7c 6a 1b 78 mr r10,r3
c096b5f8: 7c 60 00 a6 mfmsr r3
c096b5fc: 7c 00 01 46 wrteei 0
c096b600: 39 00 ff ff li r8,-1
-c096b604: 7d 20 50 29 lwarx r9,0,r10,1
+c096b604: 7d 20 50 28 lwarx r9,0,r10
c096b608: 2c 09 00 00 cmpwi r9,0
c096b60c: 40 82 00 10 bne c096b61c <_raw_write_lock_irqsave+0x2c>
c096b610: 7d 00 51 2d stwcx. r8,0,r10
Le 25/07/2022 à 14:52, Pali Rohár a écrit :
> On Monday 25 July 2022 18:20:01 Michael Ellerman wrote:
>> Pali Rohár <[email protected]> writes:
>>> On Saturday 23 July 2022 14:42:22 Christophe Leroy wrote:
>>>> Le 22/07/2022 à 11:09, Pali Rohár a écrit :
>>>>> Trying to boot mainline Linux kernel v5.15+, including current version
>>>>> from master branch, on Freescale P2020 does not work. Kernel does not
>>>>> print anything to serial console, seems that it does not work and after
>>>>> timeout watchdog reset the board.
>>>>
>>>> Can you provide more information ? Which defconfig or .config, which
>>>> version of gcc, etc ... ?
>>>
>>> I used default defconfig for mpc85xx with gcc 8, compilation for e500
>>> cores.
>>>
>>> If you need exact .config content I can send it during week.
>>>
>>>>> I run git bisect and it found following commit:
>>>>>
>>>>> 9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
>>>>> commit 9401f4e46cf6965e23738f70e149172344a01eef
>>>>> Author: Christophe Leroy <[email protected]>
>>>>> Date: Tue Mar 2 08:48:11 2021 +0000
>>>>>
>>>>> powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
>>>>>
>>>>> Force the eh flag at 0 on PPC32.
>>>>>
>>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>>> Signed-off-by: Michael Ellerman <[email protected]>
>>>>> Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
>>>>>
>>>>> :040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
>>>>>
>>>>>
>>>>> If I revert this commit then kernel boots correctly. It also boots fine
>>>>> if I revert this commit on top of master branch.
>>>>>
>>>>> Freescale P2020 has two 32-bit e500 powerpc cores.
>>>>>
>>>>> Any idea why above commit is causing crash of the kernel? And why it is
>>>>> needed? Could eh flag set to 0 cause deadlock?
>>>>
>>>> Setting the eh flag to 0 is not supposed to be a change introduced by
>>>> that commit. Indeed that commit is not supposed to change anything at
>>>> all in the generated code.
>>>
>>> My understanding of that commit is that it changed eh flag parameter
>>> from 1 to 0 for 32-bit powerpc, including also p2020.
>>
>> Can you compare the disassembly before and after and find a place where
>> an instruction has changed?
>>
>> cheers
>
> Yes, of course. Here is diff between output from objdump -d vmlinux.
> original version --- is from git master branch and modified version +++
> is the original version with reverted above problematic commit.
> So the +++ version is the one which is working.
>
> --- vmlinux.master.dump 2022-07-25 14:43:45.922239496 +0200
> +++ vmlinux.revert.dump 2022-07-25 14:43:49.238259296 +0200
> @@ -1,5 +1,5 @@
>
> -vmlinux.master: file format elf32-powerpc
> +vmlinux.revert: file format elf32-powerpc
>
>
> Disassembly of section .head.text:
> @@ -11213,7 +11213,7 @@ c000b850: 3f a0 c1 0f lis r29,-1611
> c000b854: 81 02 00 04 lwz r8,4(r2)
> c000b858: 3b fd 10 68 addi r31,r29,4200
> c000b85c: 39 40 00 01 li r10,1
> -c000b860: 7d 20 f8 29 lwarx r9,0,r31,1
> +c000b860: 7d 20 f8 28 lwarx r9,0,r31
> c000b864: 2c 09 00 00 cmpwi r9,0
> c000b868: 40 82 00 10 bne c000b878 <die+0x68>
> c000b86c: 7d 40 f9 2d stwcx. r10,0,r31
That's really strange. I made a try with mpc85xx_defconfig with GCC 11
and I don't get any such difference.
Does your version of GCC has anything special ?
Can you send you exact .config ?
Thanks
Christophe
On Monday 25 July 2022 16:20:49 Christophe Leroy wrote:
> Le 25/07/2022 à 14:52, Pali Rohár a écrit :
> > On Monday 25 July 2022 18:20:01 Michael Ellerman wrote:
> >> Pali Rohár <[email protected]> writes:
> >>> On Saturday 23 July 2022 14:42:22 Christophe Leroy wrote:
> >>>> Le 22/07/2022 à 11:09, Pali Rohár a écrit :
> >>>>> Trying to boot mainline Linux kernel v5.15+, including current version
> >>>>> from master branch, on Freescale P2020 does not work. Kernel does not
> >>>>> print anything to serial console, seems that it does not work and after
> >>>>> timeout watchdog reset the board.
> >>>>
> >>>> Can you provide more information ? Which defconfig or .config, which
> >>>> version of gcc, etc ... ?
> >>>
> >>> I used default defconfig for mpc85xx with gcc 8, compilation for e500
> >>> cores.
> >>>
> >>> If you need exact .config content I can send it during week.
> >>>
> >>>>> I run git bisect and it found following commit:
> >>>>>
> >>>>> 9401f4e46cf6965e23738f70e149172344a01eef is the first bad commit
> >>>>> commit 9401f4e46cf6965e23738f70e149172344a01eef
> >>>>> Author: Christophe Leroy <[email protected]>
> >>>>> Date: Tue Mar 2 08:48:11 2021 +0000
> >>>>>
> >>>>> powerpc: Use lwarx/ldarx directly instead of PPC_LWARX/LDARX macros
> >>>>>
> >>>>> Force the eh flag at 0 on PPC32.
> >>>>>
> >>>>> Signed-off-by: Christophe Leroy <[email protected]>
> >>>>> Signed-off-by: Michael Ellerman <[email protected]>
> >>>>> Link: https://lore.kernel.org/r/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.leroy@csgroup.eu
> >>>>>
> >>>>> :040000 040000 fe6747e45736dfcba74914a9445e5f70f5120600 96358d08b65d3200928a973efb5b969b3d45f2b0 M arch
> >>>>>
> >>>>>
> >>>>> If I revert this commit then kernel boots correctly. It also boots fine
> >>>>> if I revert this commit on top of master branch.
> >>>>>
> >>>>> Freescale P2020 has two 32-bit e500 powerpc cores.
> >>>>>
> >>>>> Any idea why above commit is causing crash of the kernel? And why it is
> >>>>> needed? Could eh flag set to 0 cause deadlock?
> >>>>
> >>>> Setting the eh flag to 0 is not supposed to be a change introduced by
> >>>> that commit. Indeed that commit is not supposed to change anything at
> >>>> all in the generated code.
> >>>
> >>> My understanding of that commit is that it changed eh flag parameter
> >>> from 1 to 0 for 32-bit powerpc, including also p2020.
> >>
> >> Can you compare the disassembly before and after and find a place where
> >> an instruction has changed?
> >>
> >> cheers
> >
> > Yes, of course. Here is diff between output from objdump -d vmlinux.
> > original version --- is from git master branch and modified version +++
> > is the original version with reverted above problematic commit.
> > So the +++ version is the one which is working.
> >
> > --- vmlinux.master.dump 2022-07-25 14:43:45.922239496 +0200
> > +++ vmlinux.revert.dump 2022-07-25 14:43:49.238259296 +0200
> > @@ -1,5 +1,5 @@
> >
> > -vmlinux.master: file format elf32-powerpc
> > +vmlinux.revert: file format elf32-powerpc
> >
> >
> > Disassembly of section .head.text:
> > @@ -11213,7 +11213,7 @@ c000b850: 3f a0 c1 0f lis r29,-1611
> > c000b854: 81 02 00 04 lwz r8,4(r2)
> > c000b858: 3b fd 10 68 addi r31,r29,4200
> > c000b85c: 39 40 00 01 li r10,1
> > -c000b860: 7d 20 f8 29 lwarx r9,0,r31,1
> > +c000b860: 7d 20 f8 28 lwarx r9,0,r31
> > c000b864: 2c 09 00 00 cmpwi r9,0
> > c000b868: 40 82 00 10 bne c000b878 <die+0x68>
> > c000b86c: 7d 40 f9 2d stwcx. r10,0,r31
>
> That's really strange. I made a try with mpc85xx_defconfig with GCC 11
> and I don't get any such difference.
Yes, that is strange...
> Does your version of GCC has anything special ?
Nothing. Ordinary Debian 10 amd64 system with cross compiler from
gcc-powerpc-linux-gnuspe package (standard version, part of Debian).
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.
Just to note that I had to apply Makefile patch for CONFIG_E500
https://lore.kernel.org/linuxppc-dev/[email protected]/
But I was told that this issue is reproducible also by OpenWRT non-SPE
gcc 8 toolchain, without using above Makefile patch.
So I have feeling that this is either related to gcc 8 or to binutils.
On that Debian is binutils 2.31.1-16. Or maybe something in .config?
> Can you send you exact .config ?
.config from the above test case is in the attachment.
> Thanks
> Christophe
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.
The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
implementations actually raise an illegal insn exception on EH=1. It
appears P2020 is one of those.
Segher
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?
> The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> implementations actually raise an illegal insn exception on EH=1. It
> appears P2020 is one of those.
>
>
> Segher
P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
But in official Freescale/NXP documentation for e500 is documented that
lwarx supports also eh=1. Maybe it is not really supported.
https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
At least there is NOTE:
Some older processors may treat EH=1 as an illegal instruction.
On Tue, Jul 26, 2022 at 10:34 AM Pali Rohár <[email protected]> wrote:
> 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 EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> > implementations actually raise an illegal insn exception on EH=1. It
> > appears P2020 is one of those.
> >
>
> P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
> But in official Freescale/NXP documentation for e500 is documented that
> lwarx supports also eh=1. Maybe it is not really supported.
> https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
> At least there is NOTE:
> Some older processors may treat EH=1 as an illegal instruction.
In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on ppc32")
this was clarified to affect (all?) e500v1/v2, this one apparently
fixed it before,
but Christophe's commit effectively reverted that change.
I think only the simple_spinlock.h file actually uses EH=1 and this is not
included in non-SMP kernels, so presumably the only affected machines were
the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
explain why nobody noticed for the past 9 months.
Arnd
On Tue, Jul 26, 2022 at 11:02:59AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 26, 2022 at 10:34 AM Pali Roh?r <[email protected]> wrote:
> > On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:
> > > The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> > > implementations actually raise an illegal insn exception on EH=1. It
> > > appears P2020 is one of those.
> >
> > P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
> > But in official Freescale/NXP documentation for e500 is documented that
> > lwarx supports also eh=1. Maybe it is not really supported.
> > https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
(page 6-186)
> > At least there is NOTE:
> > Some older processors may treat EH=1 as an illegal instruction.
And the architecture says
Programming Note
Warning: On some processors that comply with versions of the
architecture that precede Version 2.00, executing a Load And Reserve
instruction in which EH = 1 will cause the illegal instruction error
handler to be invoked.
> In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on ppc32")
> this was clarified to affect (all?) e500v1/v2,
e500v1/v2 based chips will treat any reserved field being set in an
opcode as illegal.
while the architecture says
Reserved fields in instructions are ignored by the processor.
Whoops :-) We need fixes for processor implementation bugs all the
time of course, but this is a massive *design* bug. I'm surprised this
CPU still works as well as it does!
Even the venerable PEM (last updated in 1997) shows the EH field as
reserved, always treated as 0.
> this one apparently
> fixed it before,
> but Christophe's commit effectively reverted that change.
>
> I think only the simple_spinlock.h file actually uses EH=1
That's right afaics.
> and this is not
> included in non-SMP kernels, so presumably the only affected machines were
> the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
> explain why nobody noticed for the past 9 months.
Also people using an SMP kernel on older cores should see the problem,
no? Or is that patched out? Or does this use case never happen :-)
Segher
On Tuesday 26 July 2022 08:44:05 Segher Boessenkool wrote:
> On Tue, Jul 26, 2022 at 11:02:59AM +0200, Arnd Bergmann wrote:
> > On Tue, Jul 26, 2022 at 10:34 AM Pali Rohár <[email protected]> wrote:
> > > On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:
> > > > The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> > > > implementations actually raise an illegal insn exception on EH=1. It
> > > > appears P2020 is one of those.
> > >
> > > P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
> > > But in official Freescale/NXP documentation for e500 is documented that
> > > lwarx supports also eh=1. Maybe it is not really supported.
> > > https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
>
> (page 6-186)
>
> > > At least there is NOTE:
> > > Some older processors may treat EH=1 as an illegal instruction.
>
> And the architecture says
> Programming Note
> Warning: On some processors that comply with versions of the
> architecture that precede Version 2.00
But e500v2 is 2.03 and not older than 2.00...
> executing a Load And Reserve
> instruction in which EH = 1 will cause the illegal instruction error
> handler to be invoked.
>
> > In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on ppc32")
> > this was clarified to affect (all?) e500v1/v2,
>
> e500v1/v2 based chips will treat any reserved field being set in an
> opcode as illegal.
>
> while the architecture says
>
> Reserved fields in instructions are ignored by the processor.
>
> Whoops :-) We need fixes for processor implementation bugs all the
> time of course, but this is a massive *design* bug.
I looked also in e500v2 and P2020 errata documents there is nothing
mentioned about eh flag. But it looks like a bug.
> I'm surprised this
> CPU still works as well as it does!
>
> Even the venerable PEM (last updated in 1997) shows the EH field as
> reserved, always treated as 0.
>
> > this one apparently
> > fixed it before,
> > but Christophe's commit effectively reverted that change.
> >
> > I think only the simple_spinlock.h file actually uses EH=1
>
> That's right afaics.
>
> > and this is not
> > included in non-SMP kernels, so presumably the only affected machines were
> > the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
> > explain why nobody noticed for the past 9 months.
>
> Also people using an SMP kernel on older cores should see the problem,
> no?
Probably yes.
But most people on these machines are using stable LTS kernels and do
not upgrade too often.
So you need to wait longer time to see people starting reporting such
bugs. Need to wait at least when v4.14 and v4.19 LTS versions stops
receiving updates. v4.19 is used in Debian 10 (oldstable) and v5.4 is
used by current OpenWRT. Both distributions are still supported, so
users have not migrated to new v5.15 problematic kernel yet.
> Or is that patched out? Or does this use case never happen :-)
>
>
> Segher
On Tue, Jul 26, 2022 at 04:01:00PM +0200, Pali Roh?r wrote:
> On Tuesday 26 July 2022 08:44:05 Segher Boessenkool wrote:
> > And the architecture says
> > Programming Note
> > Warning: On some processors that comply with versions of the
> > architecture that precede Version 2.00
>
> But e500v2 is 2.03 and not older than 2.00...
Yes. And it does not implement reserved fields in instructions (*any*
reserved fields in instructions, apparently!) correctly at all.
> > e500v1/v2 based chips will treat any reserved field being set in an
> > opcode as illegal.
> >
> > while the architecture says
> >
> > Reserved fields in instructions are ignored by the processor.
> >
> > Whoops :-) We need fixes for processor implementation bugs all the
> > time of course, but this is a massive *design* bug.
>
> I looked also in e500v2 and P2020 errata documents there is nothing
> mentioned about eh flag. But it looks like a bug.
The bug is if it does this for any reserved field (and it apparently
does it for all even).
> > Also people using an SMP kernel on older cores should see the problem,
> > no?
>
> Probably yes.
>
> But most people on these machines are using stable LTS kernels and do
> not upgrade too often.
Yeah.
> So you need to wait longer time to see people starting reporting such
> bugs. Need to wait at least when v4.14 and v4.19 LTS versions stops
> receiving updates. v4.19 is used in Debian 10 (oldstable) and v5.4 is
> used by current OpenWRT. Both distributions are still supported, so
> users have not migrated to new v5.15 problematic kernel yet.
That's not a reasonable timeline for kernel development of course.
We see the same thing with the compiler... Although GCC has a much
slower release cadence (one new major version every year), it often
takes two or three or more years before we get bug reports that
something was broken. If stuff isn't tested, we cannot really support
it at all.
Segher
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
Le 26/07/2022 à 15:44, Segher Boessenkool a écrit :
> On Tue, Jul 26, 2022 at 11:02:59AM +0200, Arnd Bergmann wrote:
>> On Tue, Jul 26, 2022 at 10:34 AM Pali Rohár <[email protected]> wrote:
>>> On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:
>>>> The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
>>>> implementations actually raise an illegal insn exception on EH=1. It
>>>> appears P2020 is one of those.
>>>
>>> P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
>>> But in official Freescale/NXP documentation for e500 is documented that
>>> lwarx supports also eh=1. Maybe it is not really supported.
>>> https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
>
> (page 6-186)
>
>>> At least there is NOTE:
>>> Some older processors may treat EH=1 as an illegal instruction.
>
> And the architecture says
> Programming Note
> Warning: On some processors that comply with versions of the
> architecture that precede Version 2.00, executing a Load And Reserve
> instruction in which EH = 1 will cause the illegal instruction error
> handler to be invoked.
>
>> In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on ppc32")
>> this was clarified to affect (all?) e500v1/v2,
>
> e500v1/v2 based chips will treat any reserved field being set in an
> opcode as illegal.
>
> while the architecture says
>
> Reserved fields in instructions are ignored by the processor.
>
> Whoops :-) We need fixes for processor implementation bugs all the
> time of course, but this is a massive *design* bug. I'm surprised this
> CPU still works as well as it does!
"Programming Environments Manual for 32-Bit Implementations of the
PowerPC™ Architecture" §4.1.2.2.2 says: "Invalid forms result when a bit
or operand is coded incorrectly, for example, or when a reserved bit
(shown as ‘0’) is coded as ‘1’."
>
> Even the venerable PEM (last updated in 1997) shows the EH field as
> reserved, always treated as 0.
>
>> this one apparently
>> fixed it before,
>> but Christophe's commit effectively reverted that change.
>>
>> I think only the simple_spinlock.h file actually uses EH=1
>
> That's right afaics.
>
>> and this is not
>> included in non-SMP kernels, so presumably the only affected machines were
>> the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
>> explain why nobody noticed for the past 9 months.
>
> Also people using an SMP kernel on older cores should see the problem,
> no? Or is that patched out? Or does this use case never happen :-)
Maybe unlike e500, older cores ignore the EH bit and don't mind when
it's set to 1 ?
Chritophe
On Tue, Aug 2, 2022 at 8:47 AM Christophe Leroy
<[email protected]> wrote:
> Le 26/07/2022 à 15:44, Segher Boessenkool a écrit :
> > Whoops :-) We need fixes for processor implementation bugs all the
> > time of course, but this is a massive *design* bug. I'm surprised this
> > CPU still works as well as it does!
>
> "Programming Environments Manual for 32-Bit Implementations of the
> PowerPC™ Architecture" §4.1.2.2.2 says: "Invalid forms result when a bit
> or operand is coded incorrectly, for example, or when a reserved bit
> (shown as ‘0’) is coded as ‘1’."
> >
> > Also people using an SMP kernel on older cores should see the problem,
> > no? Or is that patched out? Or does this use case never happen :-)
It doesn't get patched out, I think it's just not a combination that anyone
tests on. The few defconfig files for SMP 85xx tend to be e500mc (which
is incompatible with the older cores).
> Maybe unlike e500, older cores ignore the EH bit and don't mind when
> it's set to 1 ?
Pretty sure this is the case. My interpretation is that Freescale and IBM
just interpreted the spec differently at the time and were not even aware
of the difference until it was too late.
Arnd