Message ID | 20230510101857.2953955-4-suagrfillet@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp3521818vqo; Wed, 10 May 2023 03:47:56 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6j4GNA0I4wvv9A1RIdwiH0dp67kvmNzy8OUEl6Ka4Gyg026XP+7yjLtZLzGKWwXnm5Yk8D X-Received: by 2002:a17:902:ed8e:b0:1a9:765c:77ac with SMTP id e14-20020a170902ed8e00b001a9765c77acmr17248683plj.7.1683715676276; Wed, 10 May 2023 03:47:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683715676; cv=none; d=google.com; s=arc-20160816; b=pJjZJobKyfm5mrgHsMeXou8oMF/IuPsIiBMKT4X0I32NXXSLP4BQ+HGiyShya98L3H KryduZy89IMTnKdrd2kr5b0n/1EZIb/XEv7HgJulA2B7xnhSCWnNHtCR/OY4LVLxQk7D 70BOZTwcoKRg1l4SF6UHMyy84GyqK6xlIC4/nTpblh1jZ3fwY3pnCLbhg901rjGenwsZ SFdIA/fNIcrRf/4XnOvcg41BI851ZPBcrnwStJtC8YThsKx+YjM28zchdWr2l9ZpUOto 99bfxIEfJsRb/cYgY3obghB57koE6jnhEKUzcV37d2Cu1XdZ7945a/GIuHG9FSGA5q/4 gRGw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=S5vFEEjmnopDHc0xjFlK98v5d+a6ehUkb1OxTzirFXs=; b=cQTGZBjBwKx9B/l7u5zXvKVhv1KmbpGXnAobH7HiYBW6Vtrc3xw+/EyUOxEb+6GB/f YhmaFAgjpBwzRdeEWNMPEf3Wts0wDHfDpVhjB80qXSAj785aqls+HdHcbwdH0GlMYODu d5wlKqsADZE1LfU5U5xUrayFdlSYFKAZq7ZTOdKhSpfpcVDBjD5PVd9e9TwG9qrYhhxv Udl262Oi5FGPETuTd6fpNhU6lxzxb2VxHXpKDbrYMkhSYdeyHcBrAdEv5x60XTx1dmXp KaR3kgiQqtjslLz+DsoBM9PiCx2+oMsfEQfZZIuHP0C7zyY6/x01/urW62Ve5ICrl8O4 eHHQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=MycZHAKq; 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; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id w1-20020a170902e88100b001a63d07af3csi3935063plg.41.2023.05.10.03.47.41; Wed, 10 May 2023 03:47:56 -0700 (PDT) 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; dkim=pass header.i=@gmail.com header.s=20221208 header.b=MycZHAKq; 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; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236538AbjEJKTh (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com> + 99 others); Wed, 10 May 2023 06:19:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236566AbjEJKTb (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 10 May 2023 06:19:31 -0400 Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E3C2176B8; Wed, 10 May 2023 03:19:29 -0700 (PDT) Received: by mail-pg1-x529.google.com with SMTP id 41be03b00d2f7-52c30fa5271so3617647a12.0; Wed, 10 May 2023 03:19:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683713969; x=1686305969; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=S5vFEEjmnopDHc0xjFlK98v5d+a6ehUkb1OxTzirFXs=; b=MycZHAKqtatZ4GlFbUi6sg2PZR12Yb/VQzOokoxdbGJHe+MlTAHBhFPtZ5BKPygoBK B2MeyhOrY0UaP/fmkmLdjUOaHYR6A1E/TdGzjOaXlYa0H2ewEKI2FavlI9UYveb9lbb3 SJJNR5WOBTnan1uhg8+i/AidIcRCJ6WKOryfBJEmkUNiX0Yd70C827SLJFmVRCxlRFUi DKCGIkPzqO5LDRRV9kBNFpQWKxYWqMWV3ZVEV1D6ygubMSAWe1GBwksgIkU2yw038UKb YCuYCnFd2GxkCM4Egf38kWeZV3K/GO7MCzXIRDTa7c0m5ZRgxISdHfOqrN0odure0JmN h0Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683713969; x=1686305969; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=S5vFEEjmnopDHc0xjFlK98v5d+a6ehUkb1OxTzirFXs=; b=ZQPql6dB+jVSydxAL0PJEUmeTSFqiYUKdUaEtOw0bcQLuSrRgMt+KhXR42+Rp6H/0Z v9Fjv36cSkFXHec8yUWjRiUhKbruQtP8eqKerMkOpeLCU4vFwsJxlFXGgLAY47nCkeb3 omZAmJ0SejDNGkjlgG31PNG8kfYFMusnL2G/iS8ZwYYoYbh++/gxZaya1O1cKi6mnjha 6O/QfTinz2Q6U/RYX7ErZVplbAvIAPbAkr7Qc8Mzs2hAdcTdhfge27EPpLlMhToVte4T W1aaauSzUCjbnbjeB+6R/D97oFdsjL+FOmE+/QlswQSYr0ZssHMtGmzcF3GDe4AVJI57 JjGg== X-Gm-Message-State: AC+VfDzu+WlHVWUYQmRbLK9UtaBOnCgoyNzOlvElcDENNbuFwhDmAhPL FCo+mMMvM0NkKZPT+AEe1ZI= X-Received: by 2002:a17:902:cece:b0:1a8:626:6d9d with SMTP id d14-20020a170902cece00b001a806266d9dmr22665000plg.62.1683713969110; Wed, 10 May 2023 03:19:29 -0700 (PDT) Received: from localhost.localdomain ([221.226.144.218]) by smtp.gmail.com with ESMTPSA id p16-20020a170902e75000b001aadd0d7364sm3393419plf.83.2023.05.10.03.19.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 May 2023 03:19:28 -0700 (PDT) From: Song Shuai <suagrfillet@gmail.com> To: paul.walmsley@sifive.com, palmer@dabbelt.com, aou@eecs.berkeley.edu, rostedt@goodmis.org, mhiramat@kernel.org, mark.rutland@arm.com, guoren@kernel.org, suagrfillet@gmail.com, jszhang@kernel.org, e.shatokhin@yadro.com Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support Date: Wed, 10 May 2023 18:18:56 +0800 Message-Id: <20230510101857.2953955-4-suagrfillet@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20230510101857.2953955-1-suagrfillet@gmail.com> References: <20230510101857.2953955-1-suagrfillet@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765503849020077395?= X-GMAIL-MSGID: =?utf-8?q?1765503849020077395?= |
Series |
riscv: Optimize function trace
|
|
Commit Message
Song Shuai
May 10, 2023, 10:18 a.m. UTC
This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V. select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the register_ftrace_direct[_multi] interfaces allowing users to register the customed trampoline (direct_caller) as the mcount for one or more target functions. And modify_ftrace_direct[_multi] are also provided for modifying direct_caller. To make the direct_caller and the other ftrace hooks (eg. function/fgraph tracer, k[ret]probes) co-exist, a temporary register is nominated to store the address of direct_caller in ftrace_regs_caller. After the setting of the address direct_caller by direct_ops->func and the RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to by the `jr` inst. Signed-off-by: Song Shuai <suagrfillet@gmail.com> Tested-by: Guo Ren <guoren@kernel.org> Signed-off-by: Guo Ren <guoren@kernel.org> --- arch/riscv/Kconfig | 1 + arch/riscv/include/asm/ftrace.h | 8 ++++++++ arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++ 3 files changed, 19 insertions(+)
Comments
Song Shuai <suagrfillet@gmail.com> writes: > This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V. > > select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the > register_ftrace_direct[_multi] interfaces allowing users to register > the customed trampoline (direct_caller) as the mcount for one or > more target functions. And modify_ftrace_direct[_multi] are also > provided for modifying direct_caller. > > To make the direct_caller and the other ftrace hooks (eg. function/fgraph > tracer, k[ret]probes) co-exist, a temporary register is nominated to > store the address of direct_caller in ftrace_regs_caller. After the > setting of the address direct_caller by direct_ops->func and the > RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to > by the `jr` inst. > > Signed-off-by: Song Shuai <suagrfillet@gmail.com> > Tested-by: Guo Ren <guoren@kernel.org> > Signed-off-by: Guo Ren <guoren@kernel.org> > --- > arch/riscv/Kconfig | 1 + > arch/riscv/include/asm/ftrace.h | 8 ++++++++ > arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++ > 3 files changed, 19 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index e0632493482f..fdf0b219a02c 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -144,6 +144,7 @@ config RISCV > select UACCESS_MEMCPY if !MMU > select ZONE_DMA32 if 64BIT > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE) > + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 84f856a3286e..84904c1e4369 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -114,6 +114,14 @@ struct ftrace_regs; > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs); > #define ftrace_graph_func ftrace_graph_func > + > +static inline void > +__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > +{ > + regs->t1 = addr; > +} > +#define arch_ftrace_set_direct_caller(fregs, addr) \ > + __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > #endif /* __ASSEMBLY__ */ > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S > index f26e9f6e2fed..9d405baedb52 100644 > --- a/arch/riscv/kernel/mcount-dyn.S > +++ b/arch/riscv/kernel/mcount-dyn.S > @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller) > > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > ENTRY(ftrace_regs_caller) > + move t1, zero Please use "mv", and not "move" [1]. > SAVE_ABI_REGS 1 > PREPARE_ARGS > > @@ -239,7 +240,10 @@ ftrace_regs_call: > call ftrace_stub > > RESTORE_ABI_REGS 1 > + bnez t1,.Ldirect > jr t0 > +.Ldirect: > + jr t1 Again, while you're doing changes here, please try to align op/operands. Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support! This does not take the WITH_CALL_OPS approach Mark suggested in the v7 threads, but given that text patching story on RISC-V is still a bit sad (inconsistency in the RV tree, no specification, cannot work with preempt, ...) I'd say this approach is OK for now, and we can change to WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work. Thoughts? Björn [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions
Björn Töpel <bjorn@kernel.org> 于2023年5月11日周四 07:19写道: > > Song Shuai <suagrfillet@gmail.com> writes: > > > This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V. > > > > select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the > > register_ftrace_direct[_multi] interfaces allowing users to register > > the customed trampoline (direct_caller) as the mcount for one or > > more target functions. And modify_ftrace_direct[_multi] are also > > provided for modifying direct_caller. > > > > To make the direct_caller and the other ftrace hooks (eg. function/fgraph > > tracer, k[ret]probes) co-exist, a temporary register is nominated to > > store the address of direct_caller in ftrace_regs_caller. After the > > setting of the address direct_caller by direct_ops->func and the > > RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to > > by the `jr` inst. > > > > Signed-off-by: Song Shuai <suagrfillet@gmail.com> > > Tested-by: Guo Ren <guoren@kernel.org> > > Signed-off-by: Guo Ren <guoren@kernel.org> > > --- > > arch/riscv/Kconfig | 1 + > > arch/riscv/include/asm/ftrace.h | 8 ++++++++ > > arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++ > > 3 files changed, 19 insertions(+) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index e0632493482f..fdf0b219a02c 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -144,6 +144,7 @@ config RISCV > > select UACCESS_MEMCPY if !MMU > > select ZONE_DMA32 if 64BIT > > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE) > > + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > select HAVE_FUNCTION_GRAPH_TRACER > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > index 84f856a3286e..84904c1e4369 100644 > > --- a/arch/riscv/include/asm/ftrace.h > > +++ b/arch/riscv/include/asm/ftrace.h > > @@ -114,6 +114,14 @@ struct ftrace_regs; > > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > > struct ftrace_ops *op, struct ftrace_regs *fregs); > > #define ftrace_graph_func ftrace_graph_func > > + > > +static inline void > > +__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > > +{ > > + regs->t1 = addr; > > +} > > +#define arch_ftrace_set_direct_caller(fregs, addr) \ > > + __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) > > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > #endif /* __ASSEMBLY__ */ > > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S > > index f26e9f6e2fed..9d405baedb52 100644 > > --- a/arch/riscv/kernel/mcount-dyn.S > > +++ b/arch/riscv/kernel/mcount-dyn.S > > @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller) > > > > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > ENTRY(ftrace_regs_caller) > > + move t1, zero > > Please use "mv", and not "move" [1]. Will fix it up in the next version > > > SAVE_ABI_REGS 1 > > PREPARE_ARGS > > > > @@ -239,7 +240,10 @@ ftrace_regs_call: > > call ftrace_stub > > > > RESTORE_ABI_REGS 1 > > + bnez t1,.Ldirect > > jr t0 > > +.Ldirect: > > + jr t1 > > Again, while you're doing changes here, please try to align op/operands. > > Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support! > > This does not take the WITH_CALL_OPS approach Mark suggested in the v7 > threads, but given that text patching story on RISC-V is still a bit sad > (inconsistency in the RV tree, no specification, cannot work with > preempt, ...) I'd say this approach is OK for now, and we can change to > WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work. > > Thoughts? > > > Björn > > [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions -- Thanks, Song
Hi, On 11.05.2023 10:19, Björn Töpel wrote: > Song Shuai <suagrfillet@gmail.com> writes: > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V. >> >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the >> register_ftrace_direct[_multi] interfaces allowing users to register >> the customed trampoline (direct_caller) as the mcount for one or >> more target functions. And modify_ftrace_direct[_multi] are also >> provided for modifying direct_caller. >> >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph >> tracer, k[ret]probes) co-exist, a temporary register is nominated to >> store the address of direct_caller in ftrace_regs_caller. After the >> setting of the address direct_caller by direct_ops->func and the >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to >> by the `jr` inst. >> >> Signed-off-by: Song Shuai <suagrfillet@gmail.com> >> Tested-by: Guo Ren <guoren@kernel.org> >> Signed-off-by: Guo Ren <guoren@kernel.org> >> --- >> arch/riscv/Kconfig | 1 + >> arch/riscv/include/asm/ftrace.h | 8 ++++++++ >> arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++ >> 3 files changed, 19 insertions(+) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index e0632493482f..fdf0b219a02c 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -144,6 +144,7 @@ config RISCV >> select UACCESS_MEMCPY if !MMU >> select ZONE_DMA32 if 64BIT >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE) >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL >> select HAVE_FUNCTION_GRAPH_TRACER >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h >> index 84f856a3286e..84904c1e4369 100644 >> --- a/arch/riscv/include/asm/ftrace.h >> +++ b/arch/riscv/include/asm/ftrace.h >> @@ -114,6 +114,14 @@ struct ftrace_regs; >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, >> struct ftrace_ops *op, struct ftrace_regs *fregs); >> #define ftrace_graph_func ftrace_graph_func >> + >> +static inline void >> +__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) >> +{ >> + regs->t1 = addr; >> +} >> +#define arch_ftrace_set_direct_caller(fregs, addr) \ >> + __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ >> >> #endif /* __ASSEMBLY__ */ >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S >> index f26e9f6e2fed..9d405baedb52 100644 >> --- a/arch/riscv/kernel/mcount-dyn.S >> +++ b/arch/riscv/kernel/mcount-dyn.S >> @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller) >> >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ >> ENTRY(ftrace_regs_caller) >> + move t1, zero > > Please use "mv", and not "move" [1]. > >> SAVE_ABI_REGS 1 >> PREPARE_ARGS >> >> @@ -239,7 +240,10 @@ ftrace_regs_call: >> call ftrace_stub >> >> RESTORE_ABI_REGS 1 >> + bnez t1,.Ldirect >> jr t0 >> +.Ldirect: >> + jr t1 > > Again, while you're doing changes here, please try to align op/operands. > > Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support! > > This does not take the WITH_CALL_OPS approach Mark suggested in the v7 > threads, but given that text patching story on RISC-V is still a bit sad > (inconsistency in the RV tree, no specification, cannot work with > preempt, ...) I'd say this approach is OK for now, and we can change to > WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work. > > Thoughts? The WITH_CALL_OPS approach seems to need much more time and effort, so, yes, I'd also use this implementation of DIRECT_CALLS for now. Other improvements could wait for the future "big patching rework". FWIW, the implementation of kprobes for RISC-V has been improving gradually too, not everything was done in the first very patchset, but it was usable nonetheless. I have not tested this particular version of this DYNAMIC_FTRACE_WITH_DIRECT_CALLS series, only some previous ones - there were no outstanding problems there. The code looks sane to me. > > > Björn > > [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions > Regards, Evgenii
On Thu, May 11, 2023 at 9:30 PM Evgenii Shatokhin <e.shatokhin@yadro.com> wrote: > > Hi, > > On 11.05.2023 10:19, Björn Töpel wrote: > > Song Shuai <suagrfillet@gmail.com> writes: > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V. > >> > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the > >> register_ftrace_direct[_multi] interfaces allowing users to register > >> the customed trampoline (direct_caller) as the mcount for one or > >> more target functions. And modify_ftrace_direct[_multi] are also > >> provided for modifying direct_caller. > >> > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to > >> store the address of direct_caller in ftrace_regs_caller. After the > >> setting of the address direct_caller by direct_ops->func and the > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to > >> by the `jr` inst. > >> > >> Signed-off-by: Song Shuai <suagrfillet@gmail.com> > >> Tested-by: Guo Ren <guoren@kernel.org> > >> Signed-off-by: Guo Ren <guoren@kernel.org> > >> --- > >> arch/riscv/Kconfig | 1 + > >> arch/riscv/include/asm/ftrace.h | 8 ++++++++ > >> arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++ > >> 3 files changed, 19 insertions(+) > >> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >> index e0632493482f..fdf0b219a02c 100644 > >> --- a/arch/riscv/Kconfig > >> +++ b/arch/riscv/Kconfig > >> @@ -144,6 +144,7 @@ config RISCV > >> select UACCESS_MEMCPY if !MMU > >> select ZONE_DMA32 if 64BIT > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE) > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > >> select HAVE_FUNCTION_GRAPH_TRACER > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > >> index 84f856a3286e..84904c1e4369 100644 > >> --- a/arch/riscv/include/asm/ftrace.h > >> +++ b/arch/riscv/include/asm/ftrace.h > >> @@ -114,6 +114,14 @@ struct ftrace_regs; > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > >> struct ftrace_ops *op, struct ftrace_regs *fregs); > >> #define ftrace_graph_func ftrace_graph_func > >> + > >> +static inline void > >> +__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > >> +{ > >> + regs->t1 = addr; > >> +} > >> +#define arch_ftrace_set_direct_caller(fregs, addr) \ > >> + __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > >> > >> #endif /* __ASSEMBLY__ */ > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S > >> index f26e9f6e2fed..9d405baedb52 100644 > >> --- a/arch/riscv/kernel/mcount-dyn.S > >> +++ b/arch/riscv/kernel/mcount-dyn.S > >> @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller) > >> > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > >> ENTRY(ftrace_regs_caller) > >> + move t1, zero > > > > Please use "mv", and not "move" [1]. > > > >> SAVE_ABI_REGS 1 > >> PREPARE_ARGS > >> > >> @@ -239,7 +240,10 @@ ftrace_regs_call: > >> call ftrace_stub > >> > >> RESTORE_ABI_REGS 1 > >> + bnez t1,.Ldirect > >> jr t0 > >> +.Ldirect: > >> + jr t1 > > > > Again, while you're doing changes here, please try to align op/operands. > > > > Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support! > > > > This does not take the WITH_CALL_OPS approach Mark suggested in the v7 > > threads, but given that text patching story on RISC-V is still a bit sad > > (inconsistency in the RV tree, no specification, cannot work with > > preempt, ...) I'd say this approach is OK for now, and we can change to > > WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work. > > > > Thoughts? > > The WITH_CALL_OPS approach seems to need much more time and effort, so, > yes, I'd also use this implementation of DIRECT_CALLS for now. Other > improvements could wait for the future "big patching rework". I agree to make DIRECT_CALLS merged first, WITH_CALL_OPS is another "big patching rework". > > FWIW, the implementation of kprobes for RISC-V has been improving > gradually too, not everything was done in the first very patchset, but > it was usable nonetheless. > > I have not tested this particular version of this > DYNAMIC_FTRACE_WITH_DIRECT_CALLS series, only some previous ones - there > were no outstanding problems there. The code looks sane to me. > > > > > > > Björn > > > > [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions > > > > Regards, > Evgenii > >
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index e0632493482f..fdf0b219a02c 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -144,6 +144,7 @@ config RISCV select UACCESS_MEMCPY if !MMU select ZONE_DMA32 if 64BIT select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE) + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 84f856a3286e..84904c1e4369 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -114,6 +114,14 @@ struct ftrace_regs; void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs); #define ftrace_graph_func ftrace_graph_func + +static inline void +__arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) +{ + regs->t1 = addr; +} +#define arch_ftrace_set_direct_caller(fregs, addr) \ + __arch_ftrace_set_direct_caller(&(fregs)->regs, addr) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ #endif /* __ASSEMBLY__ */ diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index f26e9f6e2fed..9d405baedb52 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller) #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_regs_caller) + move t1, zero SAVE_ABI_REGS 1 PREPARE_ARGS @@ -239,7 +240,10 @@ ftrace_regs_call: call ftrace_stub RESTORE_ABI_REGS 1 + bnez t1,.Ldirect jr t0 +.Ldirect: + jr t1 ENDPROC(ftrace_regs_caller) ENTRY(ftrace_caller) @@ -254,3 +258,9 @@ ftrace_call: jr t0 ENDPROC(ftrace_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ + +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS +SYM_CODE_START(ftrace_stub_direct_tramp) + jr t0 +SYM_CODE_END(ftrace_stub_direct_tramp) +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */