[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20510ce03cc9463f1c9e743c1d93b939de501b53.1566219503.git.christophe.leroy@c-s.fr>
Date: Mon, 19 Aug 2019 13:06:31 +0000 (UTC)
From: Christophe Leroy <christophe.leroy@....fr>
To: Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
Michael Ellerman <mpe@...erman.id.au>,
segher@...nel.crashing.org
Cc: linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: [PATCH 3/3] powerpc: use __builtin_trap() in BUG/WARN macros.
The below exemples of use of WARN_ON() show that the result
is sub-optimal in regard of the capabilities of powerpc.
void test_warn1(unsigned long long a)
{
WARN_ON(a);
}
void test_warn2(unsigned long a)
{
WARN_ON(a);
}
void test_warn3(unsigned long a, unsigned long b)
{
WARN_ON(a < b);
}
void test_warn4(unsigned long a, unsigned long b)
{
WARN_ON(!a);
}
void test_warn5(unsigned long a, unsigned long b)
{
WARN_ON(!a && b);
}
00000000 <test_warn1>:
0: 7c 64 23 78 or r4,r3,r4
4: 31 24 ff ff addic r9,r4,-1
8: 7c 89 21 10 subfe r4,r9,r4
c: 0f 04 00 00 twnei r4,0
10: 4e 80 00 20 blr
00000014 <test_warn2>:
14: 31 23 ff ff addic r9,r3,-1
18: 7c 69 19 10 subfe r3,r9,r3
1c: 0f 03 00 00 twnei r3,0
20: 4e 80 00 20 blr
00000024 <test_warn3>:
24: 7c 84 18 10 subfc r4,r4,r3
28: 7d 29 49 10 subfe r9,r9,r9
2c: 7d 29 00 d0 neg r9,r9
30: 0f 09 00 00 twnei r9,0
34: 4e 80 00 20 blr
00000038 <test_warn4>:
38: 7c 63 00 34 cntlzw r3,r3
3c: 54 63 d9 7e rlwinm r3,r3,27,5,31
40: 0f 03 00 00 twnei r3,0
44: 4e 80 00 20 blr
00000048 <test_warn5>:
48: 2f 83 00 00 cmpwi cr7,r3,0
4c: 39 20 00 00 li r9,0
50: 41 9e 00 0c beq cr7,5c <test_warn5+0x14>
54: 7c 84 00 34 cntlzw r4,r4
58: 54 89 d9 7e rlwinm r9,r4,27,5,31
5c: 0f 09 00 00 twnei r9,0
60: 4e 80 00 20 blr
RELOCATION RECORDS FOR [__bug_table]:
OFFSET TYPE VALUE
00000000 R_PPC_ADDR32 .text+0x0000000c
0000000c R_PPC_ADDR32 .text+0x0000001c
00000018 R_PPC_ADDR32 .text+0x00000030
00000018 R_PPC_ADDR32 .text+0x00000030
00000024 R_PPC_ADDR32 .text+0x00000040
00000030 R_PPC_ADDR32 .text+0x0000005c
Using __builtin_trap() instead of inline assembly of twnei/tdnei
provides a far better result:
00000000 <test_warn1>:
0: 7c 64 23 78 or r4,r3,r4
4: 0f 04 00 00 twnei r4,0
8: 4e 80 00 20 blr
0000000c <test_warn2>:
c: 0f 03 00 00 twnei r3,0
10: 4e 80 00 20 blr
00000014 <test_warn3>:
14: 7c 43 20 08 twllt r3,r4
18: 4e 80 00 20 blr
0000001c <test_warn4>:
1c: 0c 83 00 00 tweqi r3,0
20: 4e 80 00 20 blr
00000024 <test_warn5>:
24: 2f 83 00 00 cmpwi cr7,r3,0
28: 41 9e 00 08 beq cr7,30 <test_warn5+0xc>
2c: 0c 84 00 00 tweqi r4,0
30: 4e 80 00 20 blr
RELOCATION RECORDS FOR [__bug_table]:
OFFSET TYPE VALUE
00000000 R_PPC_ADDR32 .text+0x00000004
0000000c R_PPC_ADDR32 .text+0x0000000c
00000018 R_PPC_ADDR32 .text+0x00000014
00000024 R_PPC_ADDR32 .text+0x0000001c
00000030 R_PPC_ADDR32 .text+0x0000002c
Note that we keep using an assembly text using "twi 31, 0, 0" for
inconditional traps because GCC drops all code after
__builtin_trap() when the condition is always true at build time.
In addition, this patch also fixes bugs in the BUG_ON(x) macro
which unlike WARN_ON(x) uses (x) directly as the condition by
forcing it to long instead of using !!(x). This leads to
upper part of an unsigned long long being ignored on PPC32 and
may produce bugs on PPC64 if (x) is a smaller type like an int.
Signed-off-by: Christophe Leroy <christophe.leroy@....fr>
---
arch/powerpc/include/asm/bug.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index dbf7da90f507..a229130ffcf9 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -44,14 +44,14 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
- "2:\t" PPC_LONG "1b, %0\n" \
+ "2:\t" PPC_LONG "1b - 4, %0\n" \
"\t.short %1, %2\n" \
".org 2b+%3\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
- "2:\t" PPC_LONG "1b\n" \
+ "2:\t" PPC_LONG "1b - 4\n" \
"\t.short %2\n" \
".org 2b+%3\n" \
".previous\n"
@@ -59,7 +59,8 @@
#define BUG_ENTRY(insn, flags, ...) \
__asm__ __volatile__( \
- "1: " insn "\n" \
+ insn "\n" \
+ "1:\n" \
_EMIT_BUG_ENTRY \
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
@@ -82,7 +83,9 @@
if (x) \
BUG(); \
} else { \
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x))); \
+ if (x) \
+ __builtin_trap(); \
+ BUG_ENTRY("", 0); \
} \
} while (0)
@@ -94,9 +97,9 @@
if (__ret_warn_on) \
__WARN_TAINT(TAINT_WARN); \
} else { \
- BUG_ENTRY(PPC_TLNEI " %4, 0", \
- BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
- "r" (__ret_warn_on)); \
+ if (__ret_warn_on) \
+ __builtin_trap(); \
+ BUG_ENTRY("", BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN)); \
} \
unlikely(__ret_warn_on); \
})
--
2.13.3
Powered by blists - more mailing lists