]> git.baikalelectronics.ru Git - kernel.git/commit
powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32
authorChristophe Leroy <christophe.leroy@csgroup.eu>
Tue, 13 Apr 2021 16:38:09 +0000 (16:38 +0000)
committerMichael Ellerman <mpe@ellerman.id.au>
Sat, 14 Aug 2021 12:00:01 +0000 (22:00 +1000)
commit86e8acb5231e0604d0c04b84668e4c383894abd7
tree2d09c0cd1575509900a2c25f9f4c8e6ec32fbb15
parent6e82accae6bfbbe51e3ac7c71f89cfa38abeb369
powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32

powerpc BUG_ON() and WARN_ON() are based on using twnei instruction.

For catching simple conditions like a variable having value 0, this
is efficient because it does the test and the trap at the same time.
But most conditions used with BUG_ON or WARN_ON are more complex and
forces GCC to format the condition into a 0 or 1 value in a register.
This will usually require 2 to 3 instructions.

The most efficient solution would be to use __builtin_trap() because
GCC is able to optimise the use of the different trap instructions
based on the requested condition, but this is complex if not
impossible for the following reasons:
- __builtin_trap() is a non-recoverable instruction, so it can't be
used for WARN_ON
- Knowing which line of code generated the trap would require the
analysis of DWARF information. This is not a feature we have today.

As mentioned in commit c952e9f1dbf2 ("Fix WARN_ON() on bitfield ops")
the way WARN_ON() is implemented is suboptimal. That commit also
mentions an issue with 'long long' condition. It fixed it for
WARN_ON() but the same problem still exists today with BUG_ON() on
PPC32. It will be fixed by using the generic implementation.

By using the generic implementation, gcc will naturally generate a
branch to the unconditional trap generated by BUG().

As modern powerpc implement zero-cycle branch,
that's even more efficient.

And for the functions using WARN_ON() and its return, the test
on return from WARN_ON() is now also used for the WARN_ON() itself.

On PPC64 we don't want it because we want to be able to use CFAR
register to track how we entered the code that trapped. The CFAR
register would be clobbered by the branch.

A simple test function:

unsigned long test9w(unsigned long a, unsigned long b)
{
if (WARN_ON(!b))
return 0;
return a / b;
}

Before the patch:

0000046c <test9w>:
 46c: 7c 89 00 34  cntlzw  r9,r4
 470: 55 29 d9 7e  rlwinm  r9,r9,27,5,31
 474: 0f 09 00 00  twnei   r9,0
 478: 2c 04 00 00  cmpwi   r4,0
 47c: 41 82 00 0c  beq     488 <test9w+0x1c>
 480: 7c 63 23 96  divwu   r3,r3,r4
 484: 4e 80 00 20  blr

 488: 38 60 00 00  li      r3,0
 48c: 4e 80 00 20  blr

After the patch:

00000468 <test9w>:
 468: 2c 04 00 00  cmpwi   r4,0
 46c: 41 82 00 0c  beq     478 <test9w+0x10>
 470: 7c 63 23 96  divwu   r3,r3,r4
 474: 4e 80 00 20  blr

 478: 0f e0 00 00  twui    r0,0
 47c: 38 60 00 00  li      r3,0
 480: 4e 80 00 20  blr

So we see before the patch we need 3 instructions on the likely path
to handle the WARN_ON(). With the patch the trap goes on the unlikely
path.

See below the difference at the entry of system_call_exception where
we have several BUG_ON(), allthough less impressing.

With the patch:

00000000 <system_call_exception>:
   0: 81 6a 00 84  lwz     r11,132(r10)
   4: 90 6a 00 88  stw     r3,136(r10)
   8: 71 60 00 02  andi.   r0,r11,2
   c: 41 82 00 70  beq     7c <system_call_exception+0x7c>
  10: 71 60 40 00  andi.   r0,r11,16384
  14: 41 82 00 6c  beq     80 <system_call_exception+0x80>
  18: 71 6b 80 00  andi.   r11,r11,32768
  1c: 41 82 00 68  beq     84 <system_call_exception+0x84>
  20: 94 21 ff e0  stwu    r1,-32(r1)
  24: 93 e1 00 1c  stw     r31,28(r1)
  28: 7d 8c 42 e6  mftb    r12
...
  7c: 0f e0 00 00  twui    r0,0
  80: 0f e0 00 00  twui    r0,0
  84: 0f e0 00 00  twui    r0,0

Without the patch:

00000000 <system_call_exception>:
   0: 94 21 ff e0  stwu    r1,-32(r1)
   4: 93 e1 00 1c  stw     r31,28(r1)
   8: 90 6a 00 88  stw     r3,136(r10)
   c: 81 6a 00 84  lwz     r11,132(r10)
  10: 69 60 00 02  xori    r0,r11,2
  14: 54 00 ff fe  rlwinm  r0,r0,31,31,31
  18: 0f 00 00 00  twnei   r0,0
  1c: 69 60 40 00  xori    r0,r11,16384
  20: 54 00 97 fe  rlwinm  r0,r0,18,31,31
  24: 0f 00 00 00  twnei   r0,0
  28: 69 6b 80 00  xori    r11,r11,32768
  2c: 55 6b 8f fe  rlwinm  r11,r11,17,31,31
  30: 0f 0b 00 00  twnei   r11,0
  34: 7d 8c 42 e6  mftb    r12

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b286e07fb771a664b631cd07a40b09c06f26e64b.1618331881.git.christophe.leroy@csgroup.eu
arch/powerpc/include/asm/bug.h