From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TVD_SUBJ_WIPE_DEBT, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 803E8C4338F for ; Wed, 4 Aug 2021 05:25:43 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A874160F25 for ; Wed, 4 Aug 2021 05:25:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A874160F25 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=csgroup.eu Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4GfgF12rq6z3cTg for ; Wed, 4 Aug 2021 15:25:41 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=csgroup.eu (client-ip=93.17.235.10; helo=pegase2.c-s.fr; envelope-from=christophe.leroy@csgroup.eu; receiver=) Received: from pegase2.c-s.fr (pegase2.c-s.fr [93.17.235.10]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4GfgDZ2Qy8z2yNG for ; Wed, 4 Aug 2021 15:25:14 +1000 (AEST) Received: from localhost (mailhub3.si.c-s.fr [172.26.127.67]) by localhost (Postfix) with ESMTP id 4GfgDR2pSqz9sWD; Wed, 4 Aug 2021 07:25:11 +0200 (CEST) X-Virus-Scanned: amavisd-new at c-s.fr Received: from pegase2.c-s.fr ([172.26.127.65]) by localhost (pegase2.c-s.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pwnPOfv-FcV0; Wed, 4 Aug 2021 07:25:11 +0200 (CEST) Received: from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192]) by pegase2.c-s.fr (Postfix) with ESMTP id 4GfgDR1WyJz9sVr; Wed, 4 Aug 2021 07:25:11 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id F36F68B790; Wed, 4 Aug 2021 07:25:10 +0200 (CEST) X-Virus-Scanned: amavisd-new at c-s.fr Received: from messagerie.si.c-s.fr ([127.0.0.1]) by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023) with ESMTP id La9pB5DTjAGj; Wed, 4 Aug 2021 07:25:10 +0200 (CEST) Received: from [192.168.4.90] (unknown [192.168.4.90]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 7A04A8B764; Wed, 4 Aug 2021 07:25:10 +0200 (CEST) Subject: Re: [PATCH 1/2] powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32 From: Christophe Leroy To: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman References:

Message-ID: <2325c462-07ab-4b0f-db9e-566883d5fc15@csgroup.eu> Date: Wed, 4 Aug 2021 07:25:10 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" Gentle ping Michael ? Le 25/06/2021 à 16:41, Christophe Leroy a écrit : > Hi Michael, > > What happened to this series ? It has been flagged 'under review' in Patchwork since mid April but I > never saw it in next-test. > > Thanks > Christophe > > Le 12/04/2021 à 18:26, Christophe Leroy a écrit : >> 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 8d4fbcfbe0a4 ("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 : >> 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 >> 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 : >> 468: 2c 04 00 00 cmpwi r4,0 >> 46c: 41 82 00 0c beq 478 >> 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 : >> 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 >> 10: 71 60 40 00 andi. r0,r11,16384 >> 14: 41 82 00 6c beq 80 >> 18: 71 6b 80 00 andi. r11,r11,32768 >> 1c: 41 82 00 68 beq 84 >> 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 : >> 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 >> --- >> arch/powerpc/include/asm/bug.h | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h >> index d1635ffbb179..101dea4eec8d 100644 >> --- a/arch/powerpc/include/asm/bug.h >> +++ b/arch/powerpc/include/asm/bug.h >> @@ -68,7 +68,11 @@ >> BUG_ENTRY("twi 31, 0, 0", 0); \ >> unreachable(); \ >> } while (0) >> +#define HAVE_ARCH_BUG >> + >> +#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags)) >> +#ifdef CONFIG_PPC64 >> #define BUG_ON(x) do { \ >> if (__builtin_constant_p(x)) { \ >> if (x) \ >> @@ -78,8 +82,6 @@ >> } \ >> } while (0) >> -#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags)) >> - >> #define WARN_ON(x) ({ \ >> int __ret_warn_on = !!(x); \ >> if (__builtin_constant_p(__ret_warn_on)) { \ >> @@ -93,9 +95,10 @@ >> unlikely(__ret_warn_on); \ >> }) >> -#define HAVE_ARCH_BUG >> #define HAVE_ARCH_BUG_ON >> #define HAVE_ARCH_WARN_ON >> +#endif >> + >> #endif /* __ASSEMBLY __ */ >> #else >> #ifdef __ASSEMBLY__ >>