Received: by 2002:a05:6a10:7420:0:0:0:0 with SMTP id hk32csp62906pxb; Tue, 15 Feb 2022 08:29:34 -0800 (PST) X-Google-Smtp-Source: ABdhPJwuijUY+SqBNUcKG81w/TupCp3+sc9i3jfw96acmAqXHcfAtiCvLrQw1U7zReK+X3gDpGcq X-Received: by 2002:aa7:d8ce:: with SMTP id k14mr2499779eds.407.1644942574731; Tue, 15 Feb 2022 08:29:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1644942574; cv=none; d=google.com; s=arc-20160816; b=khkYmemXi5pC7Il7WV13zrmtEOY/8+QJ1uZCuESA8DxrCi4lX8DVCPowFVreOVa3Mh sLSOLSq3WMgsi0Whe1u6FghkNoI2V+b0tfd2pkXVq+y5UmEnnXKDHSpjJdx76F0PG7QV S+/nymP2vwWiej0fWyUwQ4ZpUDeq1wYnvV/rvw0T4ptUkOpjey6wRvKkfaiUndEY31Dm ztlQ1ObeCXUy66AdUADEoFinAShyK/OH2hG2pCwllqkmXnb5h37+jzvjeuawzKe1Dbvb 3FpxFWcsxeBFjwTE3upfMI6Mj93Kq2bDCudki36sYlHL+aJLbQRt5iHcCRMDNciZGEZA q24w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:references :cc:to:from:content-language:subject:user-agent:mime-version:date :message-id; bh=bDzECpoT7yCKuGmVfyvt5HoIq/ygo/ysu2pphfEW8E8=; b=MK7JbJUED/QeUCJqaLA69xGLaJTEqsMYa2IyZ7oa6w9bGxe+yz8iv8nIEyF98vgt4H pbFV8zNSQzFrnShMMWwnOTDH7aYOlrHfZbY5kkJkvQHUUtWV8uxGAoRaonroRiudEKuK 8MdsZwX24tDGIRuWemQXGUG6brL8w0juO4a1ZU1/Xx7jL4SzQmhlKOcdgAHiishsbwNd GpHN7d9d0pIielQk98eXDkJ8YyK1egc5sTMD01m4Mj2s1z4q7nVovSSYhf+x8O+9qTg5 Qk3JgvQaeGnYCjzyDembdFje3BHDxoRHt2DYBI+1qcX31KirqGJ9NoZ+SX8BY9wOURxU F6QA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id r18si183391edd.183.2022.02.15.08.28.50; Tue, 15 Feb 2022 08:29:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238602AbiBOPUW (ORCPT + 99 others); Tue, 15 Feb 2022 10:20:22 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:53316 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234307AbiBOPUV (ORCPT ); Tue, 15 Feb 2022 10:20:21 -0500 X-Greylist: delayed 1689 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 15 Feb 2022 07:20:11 PST Received: from pegase2.c-s.fr (pegase2.c-s.fr [93.17.235.10]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF6B4108772; Tue, 15 Feb 2022 07:20:10 -0800 (PST) Received: from localhost (mailhub3.si.c-s.fr [172.26.127.67]) by localhost (Postfix) with ESMTP id 4JykZR3vl3z9sSC; Tue, 15 Feb 2022 15:51:59 +0100 (CET) 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 IzdyMfHEghQL; Tue, 15 Feb 2022 15:51:59 +0100 (CET) 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 4JykZR2pRtz9sQv; Tue, 15 Feb 2022 15:51:59 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 4E19B8B778; Tue, 15 Feb 2022 15:51:59 +0100 (CET) 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 QWce0lNxMQK9; Tue, 15 Feb 2022 15:51:59 +0100 (CET) Received: from [192.168.6.174] (unknown [192.168.6.174]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 7DA128B763; Tue, 15 Feb 2022 15:51:58 +0100 (CET) Message-ID: <5c7b5334-6071-f131-a509-9a49ca3d628c@csgroup.eu> Date: Tue, 15 Feb 2022 15:51:58 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 09/13] powerpc/ftrace: Implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS Content-Language: fr-FR From: Christophe Leroy To: "Naveen N. Rao" , Jiri Kosina , Joe Lawrence , Josh Poimboeuf , Miroslav Benes , Ingo Molnar , Michael Ellerman , Petr Mladek , Steven Rostedt , Heiko Carstens , Vasily Gorbik Cc: "linux-kernel@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , "linux-s390@vger.kernel.org" , "live-patching@vger.kernel.org" References: <5831f711a778fcd6eb51eb5898f1faae4378b35b.1640017960.git.christophe.leroy@csgroup.eu> <1644852011.qg7ud9elo2.naveen@linux.ibm.com> <1b28f52a-f8b7-6b5c-e726-feac4123517d@csgroup.eu> <875ypgo0f3.fsf@mpe.ellerman.id.au> <1644930705.g64na2kgvd.naveen@linux.ibm.com> <6dc50f09-4d14-afa2-d2a1-34b72b880edf@csgroup.eu> In-Reply-To: <6dc50f09-4d14-afa2-d2a1-34b72b880edf@csgroup.eu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org + S390 people Le 15/02/2022 à 15:28, Christophe Leroy a écrit : > > > Le 15/02/2022 à 14:36, Naveen N. Rao a écrit : >> Michael Ellerman wrote: >>> Christophe Leroy writes: >>>> Le 14/02/2022 à 16:25, Naveen N. Rao a écrit : >>>>> Christophe Leroy wrote: >>>>>> Implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS. It accelerates the call >>>>>> of livepatching. >>>>>> >>>>>> Also note that powerpc being the last one to convert to >>>>>> CONFIG_DYNAMIC_FTRACE_WITH_ARGS, it will now be possible to remove >>>>>> klp_arch_set_pc() on all architectures. >>>>>> >>>>>> Signed-off-by: Christophe Leroy >>>>>> --- >>>>>>  arch/powerpc/Kconfig                 |  1 + >>>>>>  arch/powerpc/include/asm/ftrace.h    | 17 +++++++++++++++++ >>>>>>  arch/powerpc/include/asm/livepatch.h |  4 +--- >>>>>>  3 files changed, 19 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>>>>> index cdac2115eb00..e2b1792b2aae 100644 >>>>>> --- a/arch/powerpc/Kconfig >>>>>> +++ b/arch/powerpc/Kconfig >>>>>> @@ -210,6 +210,7 @@ config PPC >>>>>>      select HAVE_DEBUG_KMEMLEAK >>>>>>      select HAVE_DEBUG_STACKOVERFLOW >>>>>>      select HAVE_DYNAMIC_FTRACE >>>>>> +    select HAVE_DYNAMIC_FTRACE_WITH_ARGS    if MPROFILE_KERNEL || >>>>>> PPC32 >>>>>>      select HAVE_DYNAMIC_FTRACE_WITH_REGS    if MPROFILE_KERNEL || >>>>>> PPC32 >>>>>>      select HAVE_EBPF_JIT >>>>>>      select HAVE_EFFICIENT_UNALIGNED_ACCESS    if >>>>>> !(CPU_LITTLE_ENDIAN && POWER7_CPU) >>>>>> diff --git a/arch/powerpc/include/asm/ftrace.h >>>>>> b/arch/powerpc/include/asm/ftrace.h >>>>>> index b3f6184f77ea..45c3d6f11daa 100644 >>>>>> --- a/arch/powerpc/include/asm/ftrace.h >>>>>> +++ b/arch/powerpc/include/asm/ftrace.h >>>>>> @@ -22,6 +22,23 @@ static inline unsigned long >>>>>> ftrace_call_adjust(unsigned long addr) >>>>>>  struct dyn_arch_ftrace { >>>>>>      struct module *mod; >>>>>>  }; >>>>>> + >>>>>> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS >>>>>> +struct ftrace_regs { >>>>>> +    struct pt_regs regs; >>>>>> +}; >>>>>> + >>>>>> +static __always_inline struct pt_regs >>>>>> *arch_ftrace_get_regs(struct ftrace_regs *fregs) >>>>>> +{ >>>>>> +    return &fregs->regs; >>>>>> +} >>>>> >>>>> I think this is wrong. We need to differentiate between >>>>> ftrace_caller() and ftrace_regs_caller() here, and only return >>>>> pt_regs if coming in through ftrace_regs_caller() (i.e., >>>>> FL_SAVE_REGS is set). >>>> >>>> Not sure I follow you. >>>> >>>> This is based on 5740a7c71ab6 ("s390/ftrace: add >>>> HAVE_DYNAMIC_FTRACE_WITH_ARGS support") >>>> >>>> It's all the point of HAVE_DYNAMIC_FTRACE_WITH_ARGS, have the regs >>>> also with ftrace_caller(). >>>> >>>> Sure you only have the params, but that's the same on s390, so what >>>> did I miss ? >> >> It looks like s390 is special since it apparently saves all registers >> even for ftrace_caller: >> https://lore.kernel.org/all/YbipdU5X4HNDWIni@osiris/ > > It is not what I understand from their code, see > https://elixir.bootlin.com/linux/v5.17-rc3/source/arch/s390/kernel/mcount.S#L37 > > > They have a common macro called with argument 'allregs' which is set to > 0 for ftrace_caller() and 1 for ftrace_regs_caller(). > When allregs == 1, the macro seems to save more. > > But ok, I can do like x86, but I need a trick to know whether > FL_SAVE_REGS is set or not, like they do with fregs->regs.cs > Any idea what the condition can be for powerpc ? > Finally, it looks like this change is done via commit 894979689d3a ("s390/ftrace: provide separate ftrace_caller/ftrace_regs_caller implementations") four hours the same day after the implementation of arch_ftrace_get_regs() They may have forgotten to change arch_ftrace_get_regs() which was added in commit 5740a7c71ab6 ("s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_ARGS support") with the assumption that ftrace_caller and ftrace_regs_caller where identical. Christophe