Message ID | 20221123142025.1504030-2-suagrfillet@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:b599:b0:88:f841:fdc9 with SMTP id df25csp2168367dyb; Wed, 23 Nov 2022 06:25:46 -0800 (PST) X-Google-Smtp-Source: AA0mqf6R6R0TsSQ9ALUQLCrOzwXwhcrP5zAOKJaPKPqiwvSwlb9C2tO1pnjzngT/oA34hDBxvoH5 X-Received: by 2002:a17:906:5409:b0:7b2:7b45:848f with SMTP id q9-20020a170906540900b007b27b45848fmr24196261ejo.129.1669213546374; Wed, 23 Nov 2022 06:25:46 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669213546; cv=none; d=google.com; s=arc-20160816; b=N0bPJ3LaDcVEhbQ5LjGvQbcxW4nTcB6M4UfEwheR/aBgCBanPnbUeNy+rVcFmpSro+ haSQ3Axh9YbSz1lD4+g79xpMe90zsTRnCLfboKgilQespFxMcFA7YF8+vVnwJzW4L9Wa 265g4WyaZ6hvgUp9biayWzjAdzFZ8MUh6X2MRw2d/O5Zvtw198T/92dVXpqzVjq+6PBI /9HgHsM4UWt3pRljXAQaCWxcwDxBACW3NZWWGhHnDo9+iYvg2iixQK3L9JMVnJIjL/mM xohh/EH1ohUpVctCCRkyiMhDtYXitpEaUMD9PkASA14JFdEYJiLjfcoa/S97bcL+VARp ywKw== 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=qxhkKuZvqbaLoVEzfi+iXfk1DD9YbSKqrCN+Rkf9lGA=; b=D3+fDdOx9WlHhHMxZTQ0RuzbFA1QOkKUwioNrVexKWz1y8edhn7WyCdzDcRSV73N5r lnaHPU9vl5ZHPqlxrgN0yKBNeVGPrUGwmcYtJvDg/QRYJsBzwL2TCXbfGtOq0B2QupHo /8v5pMDfJm0jSzgLcbJu1CTTe3MjxquCNtKemSlboDrELVVmi2MCh+GsQWtME1MMvg7E 03EHI9gyxkJE4QdTRWQUia6pUQ2cfMgFwC63j/NVi7eqopUC/rUoi4WhfFBxBxt1g5fP XIfqRtF5Nfq9DBf1P5iGgAqFYxbxRo/6ywPmZu0vlSjhlW3DByPd0GDWGlN7SktsAbqg WcIw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=mBP5+fAX; 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 b16-20020a056402279000b00459cf784343si15274299ede.176.2022.11.23.06.25.18; Wed, 23 Nov 2022 06:25:46 -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; dkim=pass header.i=@gmail.com header.s=20210112 header.b=mBP5+fAX; 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 S236923AbiKWOVh (ORCPT <rfc822;fengqi706@gmail.com> + 99 others); Wed, 23 Nov 2022 09:21:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237621AbiKWOUv (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 23 Nov 2022 09:20:51 -0500 Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 300BA65863 for <linux-kernel@vger.kernel.org>; Wed, 23 Nov 2022 06:20:39 -0800 (PST) Received: by mail-pf1-x42a.google.com with SMTP id z26so17462991pff.1 for <linux-kernel@vger.kernel.org>; Wed, 23 Nov 2022 06:20:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=qxhkKuZvqbaLoVEzfi+iXfk1DD9YbSKqrCN+Rkf9lGA=; b=mBP5+fAXxuAxdCSP7zz4tcSWzEJr0hKfXx05l9u3poc8f2lajrRDJ4om5XYRcd98cX NLSa3uq9HOym2r1qyYi9iYHEmbhSlyJKl/FNE5lZxwm0dzVo81wicYiH1ou3HUtugTqI rP4v9oD7NDJQfw2siOAoyIh7gfzl0Dgj2wrGWTbppytIC4j0Fn4EiXlXR82cRpZWKLHQ kujHqPeRV95YUrZxHUOXGG0YPe58U6En1STNS1KBld/YP6FHULqFQuAqRyTc8gkPuM+Q IlUhPgEsBIvmc5Uvd+rlFDrV3xE4ZTN2k34MiRwlB+AP0XFGtCEjv5+QbMRAUb4GpeyD gjNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=qxhkKuZvqbaLoVEzfi+iXfk1DD9YbSKqrCN+Rkf9lGA=; b=lx7C8w6tw/tZ332wVv1JpRXHXx18d2rQ5L87t5iOR+G9z0DxDMj/Hvi2O8obutjtJD n8i82QmJv9ndj+R2QgwoQWlkcwJel5NUndJHgadU9uwFjiL0O912r07IA1y9laXNknpp /8NkPOnVDqSZYYog29uHUoL9ookxJlmCX/NkSKF8o2qcMKwrNAbIqQSxBRQa52tiVtgg SuDYD6K4gNeBVU4E17Gmt8+jnho99mraP7KdX4R5VcoiOctcUYuMGUs0eQglNFsT1tUS ha6fz8FbFO6hQe/Gsaqm/Jzi+KT0htMQRzfdJl9HoNKr5xRg+d5JhOYapjgXvBhS0YQf YlPQ== X-Gm-Message-State: ANoB5pkLX9ob4dyRn4UIabV7Adii6jAj6N9Lc/fwnHEtoAVOR8wSCA0y GlQ0rEPTl+qaCdEjUXoVpR+CBj69wn0= X-Received: by 2002:a63:4909:0:b0:462:8605:7fe7 with SMTP id w9-20020a634909000000b0046286057fe7mr8277358pga.445.1669213238663; Wed, 23 Nov 2022 06:20:38 -0800 (PST) Received: from localhost.localdomain ([221.226.144.218]) by smtp.gmail.com with ESMTPSA id l12-20020a170903120c00b0016c5306917fsm14516732plh.53.2022.11.23.06.20.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Nov 2022 06:20:38 -0800 (PST) From: Song Shuai <suagrfillet@gmail.com> To: guoren@kernel.org, paul.walmsley@sifive.com, palmer@dabbelt.com, aou@eecs.berkeley.edu, rostedt@goodmis.org, mhiramat@kernel.org, mark.rutland@arm.com, peterz@infradead.org, jolsa@redhat.com, bp@suse.de, jpoimboe@kernel.org Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, Song Shuai <suagrfillet@gmail.com> Subject: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support Date: Wed, 23 Nov 2022 22:20:24 +0800 Message-Id: <20221123142025.1504030-2-suagrfillet@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20221123142025.1504030-1-suagrfillet@gmail.com> References: <20221123142025.1504030-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 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?1750297263604248362?= X-GMAIL-MSGID: =?utf-8?q?1750297263604248362?= |
Series |
riscv/ftrace: add WITH_DIRECT_CALLS support
|
|
Commit Message
Song Shuai
Nov. 23, 2022, 2:20 p.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>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/ftrace.h | 6 ++++++
arch/riscv/kernel/mcount-dyn.S | 4 ++++
3 files changed, 11 insertions(+)
Comments
Hi Song, On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > 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> > --- > arch/riscv/Kconfig | 1 + > arch/riscv/include/asm/ftrace.h | 6 ++++++ > arch/riscv/kernel/mcount-dyn.S | 4 ++++ > 3 files changed, 11 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 39ec8d628cf6..d083ec08d0b6 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -278,6 +278,7 @@ config ARCH_RV64I > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 01bebb28eabe..be4d57566139 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -114,6 +114,12 @@ 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; How about regs->t0 = addr; ? And delete all mcount-dyn.S modification. > +} > + > #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 466c6ef217b1..b89c85a58569 100644 > --- a/arch/riscv/kernel/mcount-dyn.S > +++ b/arch/riscv/kernel/mcount-dyn.S > @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > ENTRY(ftrace_regs_caller) > SAVE_ABI_REGS 1 > + REG_S x0, PT_T1(sp) > PREPARE_ARGS > > ftrace_regs_call: > @@ -241,7 +242,10 @@ ftrace_regs_call: > > > RESTORE_ABI_REGS 1 > + bnez t1,.Ldirect > jr t0 > +.Ldirect: > + jr t1 > ENDPROC(ftrace_regs_caller) > > ENTRY(ftrace_caller) > -- > 2.20.1 > -- Best Regards Guo Ren
Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > Cool job, thx. > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: >> >> 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> >> --- >> arch/riscv/Kconfig | 1 + >> arch/riscv/include/asm/ftrace.h | 6 ++++++ >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 39ec8d628cf6..d083ec08d0b6 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -278,6 +278,7 @@ config ARCH_RV64I >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL >> select HAVE_FUNCTION_GRAPH_TRACER >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h >> index 01bebb28eabe..be4d57566139 100644 >> --- a/arch/riscv/include/asm/ftrace.h >> +++ b/arch/riscv/include/asm/ftrace.h >> @@ -114,6 +114,12 @@ 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; > > How about regs->t0 = addr; ? > And delete all mcount-dyn.S modification. > The direct_caller has the same program layout as the ftrace_caller, which means the reg t0 will never be changed when direct_caller returns. If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, direct_caller will enter the dead loop. Actually the reg t0 always saves the address of function entry with 8B offset, it should only changed by the IPMODIFY ops instead of the direct_ops. >> >> +} >> + >> #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 466c6ef217b1..b89c85a58569 100644 >> --- a/arch/riscv/kernel/mcount-dyn.S >> +++ b/arch/riscv/kernel/mcount-dyn.S >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ >> ENTRY(ftrace_regs_caller) >> SAVE_ABI_REGS 1 >> + REG_S x0, PT_T1(sp) >> PREPARE_ARGS >> >> ftrace_regs_call: >> @@ -241,7 +242,10 @@ ftrace_regs_call: >> >> >> RESTORE_ABI_REGS 1 >> + bnez t1,.Ldirect >> jr t0 >> +.Ldirect: >> + jr t1 >> ENDPROC(ftrace_regs_caller) >> >> ENTRY(ftrace_caller) >> -- >> 2.20.1 >> > > > -- > Best Regards > Guo Ren
On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > Cool job, thx. > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > >> > >> 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> > >> --- > >> arch/riscv/Kconfig | 1 + > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > >> 3 files changed, 11 insertions(+) > >> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >> index 39ec8d628cf6..d083ec08d0b6 100644 > >> --- a/arch/riscv/Kconfig > >> +++ b/arch/riscv/Kconfig > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > >> select HAVE_FUNCTION_GRAPH_TRACER > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > >> index 01bebb28eabe..be4d57566139 100644 > >> --- a/arch/riscv/include/asm/ftrace.h > >> +++ b/arch/riscv/include/asm/ftrace.h > >> @@ -114,6 +114,12 @@ 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; > > > > How about regs->t0 = addr; ? > > And delete all mcount-dyn.S modification. > > > The direct_caller has the same program layout as the ftrace_caller, which means > the reg t0 will never be changed when direct_caller returns. > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > direct_caller will enter the dead loop. ? ftrace_regs_caller->call_direct_funcs-> arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0. Only call_direct_funcs call arch_ftrace_set_direct_caller ! static void call_direct_funcs(unsigned long ip, unsigned long pip, struct ftrace_ops *ops, struct ftrace_regs *fregs) { struct pt_regs *regs = ftrace_get_regs(fregs); unsigned long addr; addr = ftrace_find_rec_direct(ip); if (!addr) return; arch_ftrace_set_direct_caller(regs, addr); } > > Actually the reg t0 always saves the address of function entry with 8B > offset, it should only > changed by the IPMODIFY ops instead of the direct_ops. > >> > >> +} > >> + > >> #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 466c6ef217b1..b89c85a58569 100644 > >> --- a/arch/riscv/kernel/mcount-dyn.S > >> +++ b/arch/riscv/kernel/mcount-dyn.S > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > >> ENTRY(ftrace_regs_caller) > >> SAVE_ABI_REGS 1 > >> + REG_S x0, PT_T1(sp) > >> PREPARE_ARGS > >> > >> ftrace_regs_call: > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > >> > >> > >> RESTORE_ABI_REGS 1 > >> + bnez t1,.Ldirect > >> jr t0 > >> +.Ldirect: > >> + jr t1 > >> ENDPROC(ftrace_regs_caller) > >> > >> ENTRY(ftrace_caller) > >> -- > >> 2.20.1 > >> > > > > > > -- > > Best Regards > > Guo Ren
Guo Ren <guoren@kernel.org> 于2022年11月24日周四 02:08写道: > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > Cool job, thx. > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > >> > > >> 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> > > >> --- > > >> arch/riscv/Kconfig | 1 + > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > >> 3 files changed, 11 insertions(+) > > >> > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > >> --- a/arch/riscv/Kconfig > > >> +++ b/arch/riscv/Kconfig > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > >> select HAVE_FUNCTION_GRAPH_TRACER > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > >> index 01bebb28eabe..be4d57566139 100644 > > >> --- a/arch/riscv/include/asm/ftrace.h > > >> +++ b/arch/riscv/include/asm/ftrace.h > > >> @@ -114,6 +114,12 @@ 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; > > > > > > How about regs->t0 = addr; ? > > > And delete all mcount-dyn.S modification. > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > the reg t0 will never be changed when direct_caller returns. > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > direct_caller will enter the dead loop. > ? > > ftrace_regs_caller->call_direct_funcs-> > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0. > > Only call_direct_funcs call arch_ftrace_set_direct_caller ! > > static void call_direct_funcs(unsigned long ip, unsigned long pip, > struct ftrace_ops *ops, struct ftrace_regs *fregs) > { > struct pt_regs *regs = ftrace_get_regs(fregs); > unsigned long addr; > > addr = ftrace_find_rec_direct(ip); > if (!addr) > return; > > arch_ftrace_set_direct_caller(regs, addr); > } > When direct_caller and function tracer co-hook a function, the simple diagram is like this: ``` func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> function_trace_call // write ringbuffer | -> call_direct_funcs // regs->t1 = direct_caller -> t1 !=0 && jr t1 // goto direct_caller ``` And the direct_caller has a similar implement as ftrace_caller. ``` direct_caller: add sp,sp,-? sd t0,?(sp) sd ra,?(sp) call foo ld t0,?(sp) ld ra,?(sp) add sp,sp,? jr t0 // <- back to function entry ``` If we change regs->t0 as direct_caller and execute `jr t0` directly, the direct_caller will always jump to itself due to its last `jr` inst. ``` func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> function_trace_call // write ringbuffer | -> call_direct_funcs // regs->t0 = direct_caller -> jr t0 // goto direct_caller direct_caller: ... sd t0,?(sp) ... ld t0,?(s0) ... jr t0 // goto direct_caller always ``` Hope I made it clear. > > > > Actually the reg t0 always saves the address of function entry with 8B > > offset, it should only > > changed by the IPMODIFY ops instead of the direct_ops. > > >> > > >> +} > > >> + > > >> #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 466c6ef217b1..b89c85a58569 100644 > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > >> ENTRY(ftrace_regs_caller) > > >> SAVE_ABI_REGS 1 > > >> + REG_S x0, PT_T1(sp) > > >> PREPARE_ARGS > > >> > > >> ftrace_regs_call: > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > >> > > >> > > >> RESTORE_ABI_REGS 1 > > >> + bnez t1,.Ldirect > > >> jr t0 > > >> +.Ldirect: > > >> + jr t1 > > >> ENDPROC(ftrace_regs_caller) > > >> > > >> ENTRY(ftrace_caller) > > >> -- > > >> 2.20.1 > > >> > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > > > -- > Best Regards > Guo Ren Thanks, Song
Song Shuai <suagrfillet@gmail.com> 于2022年11月24日周四 02:52写道: > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 02:08写道: > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > Cool job, thx. > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > >> > > > >> 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> > > > >> --- > > > >> arch/riscv/Kconfig | 1 + > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > >> 3 files changed, 11 insertions(+) > > > >> > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > >> --- a/arch/riscv/Kconfig > > > >> +++ b/arch/riscv/Kconfig > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > >> index 01bebb28eabe..be4d57566139 100644 > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > How about regs->t0 = addr; ? > > > > And delete all mcount-dyn.S modification. > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > the reg t0 will never be changed when direct_caller returns. > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > direct_caller will enter the dead loop. > > ? > > > > ftrace_regs_caller->call_direct_funcs-> > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0. > > > > Only call_direct_funcs call arch_ftrace_set_direct_caller ! > > > > static void call_direct_funcs(unsigned long ip, unsigned long pip, > > struct ftrace_ops *ops, struct ftrace_regs *fregs) > > { > > struct pt_regs *regs = ftrace_get_regs(fregs); > > unsigned long addr; > > > > addr = ftrace_find_rec_direct(ip); > > if (!addr) > > return; > > > > arch_ftrace_set_direct_caller(regs, addr); > > } > > > When direct_caller and function tracer co-hook a function, the simple > diagram is like this: > > ``` > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > function_trace_call // write ringbuffer > | > -> call_direct_funcs // regs->t1 = direct_caller > -> t1 !=0 && jr t1 // goto direct_caller > ``` > ``` f -- regs_caller -- list_func | | -- function_trace_call | | -- call_direct_funcs // t1 = addr |-- t1 !=0 && jr t1 // goto direct_caller ``` This diagram looks better. > And the direct_caller has a similar implement as ftrace_caller. > > ``` > direct_caller: > add sp,sp,-? > sd t0,?(sp) > sd ra,?(sp) > call foo > ld t0,?(sp) > ld ra,?(sp) > add sp,sp,? > jr t0 // <- back to function entry > ``` > > If we change regs->t0 as direct_caller and execute `jr t0` directly, > the direct_caller will always jump to itself due to its last `jr` inst. > > ``` > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > function_trace_call // write ringbuffer > | > -> call_direct_funcs // regs->t0 = direct_caller > -> jr t0 // goto direct_caller > > direct_caller: > ... > sd t0,?(sp) > ... > ld t0,?(s0) > ... > jr t0 // goto direct_caller always > ``` > > Hope I made it clear. > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > offset, it should only > > > changed by the IPMODIFY ops instead of the direct_ops. > > > >> > > > >> +} > > > >> + > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > >> ENTRY(ftrace_regs_caller) > > > >> SAVE_ABI_REGS 1 > > > >> + REG_S x0, PT_T1(sp) > > > >> PREPARE_ARGS > > > >> > > > >> ftrace_regs_call: > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > >> > > > >> > > > >> RESTORE_ABI_REGS 1 > > > >> + bnez t1,.Ldirect > > > >> jr t0 > > > >> +.Ldirect: > > > >> + jr t1 > > > >> ENDPROC(ftrace_regs_caller) > > > >> > > > >> ENTRY(ftrace_caller) > > > >> -- > > > >> 2.20.1 > > > >> > > > > > > > > > > > > -- > > > > Best Regards > > > > Guo Ren > > > > > > > > -- > > Best Regards > > Guo Ren > Thanks, > Song Thanks, Song
On Thu, Nov 24, 2022 at 11:09 AM Song Shuai <suagrfillet@gmail.com> wrote: > > Song Shuai <suagrfillet@gmail.com> 于2022年11月24日周四 02:52写道: > > > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 02:08写道: > > > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > > > Cool job, thx. > > > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > >> > > > > >> 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> > > > > >> --- > > > > >> arch/riscv/Kconfig | 1 + > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > > >> 3 files changed, 11 insertions(+) > > > > >> > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > > >> --- a/arch/riscv/Kconfig > > > > >> +++ b/arch/riscv/Kconfig > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > >> index 01bebb28eabe..be4d57566139 100644 > > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > > > How about regs->t0 = addr; ? > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > > the reg t0 will never be changed when direct_caller returns. > > > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > > direct_caller will enter the dead loop. > > > ? > > > > > > ftrace_regs_caller->call_direct_funcs-> > > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0. > > > > > > Only call_direct_funcs call arch_ftrace_set_direct_caller ! > > > > > > static void call_direct_funcs(unsigned long ip, unsigned long pip, > > > struct ftrace_ops *ops, struct ftrace_regs *fregs) > > > { > > > struct pt_regs *regs = ftrace_get_regs(fregs); > > > unsigned long addr; > > > > > > addr = ftrace_find_rec_direct(ip); > > > if (!addr) > > > return; > > > > > > arch_ftrace_set_direct_caller(regs, addr); > > > } > > > > > When direct_caller and function tracer co-hook a function, the simple > > diagram is like this: > > > > ``` > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > > function_trace_call // write ringbuffer > > | > > -> call_direct_funcs // regs->t1 = direct_caller > > -> t1 !=0 && jr t1 // goto direct_caller > > ``` > > > ``` > f -- regs_caller -- list_func > | | -- function_trace_call > | | -- call_direct_funcs // t1 = addr > |-- t1 !=0 && jr t1 // goto direct_caller Cool diagram! Thx, but we still need a discussion: f -- regs_caller | -- SAVE_ABI_REGS 1 | -- list_func | | -- function_trace_call | | -- call_direct_funcs // t1 = addr | -- RESTORE_ABI_REGS 1 |-- t1 !=0 && jr t1 // goto direct_caller If you set t1 non-zero, then we always go to direct_caller. I think the code is equal to set t0=addr. | | -- call_direct_funcs // t0 = addr | -- RESTORE_ABI_REGS 1 |-- jr t0 // goto direct_caller I think the only problem happens in the below non-existent situation: f -- regs_caller | -- SAVE_ABI_REGS 1 | -- list_func | | -- call_direct_funcs // t0 = addr | | -- function_trace_call //t0 contains direct_caller instead | -- RESTORE_ABI_REGS 1 |-- jr t0 // goto direct_caller The key issue is whether going to direct_caller correctly depends on call_direct_funcs called, right? > ``` > This diagram looks better. > > And the direct_caller has a similar implement as ftrace_caller. > > > > ``` > > direct_caller: > > add sp,sp,-? > > sd t0,?(sp) > > sd ra,?(sp) > > call foo > > ld t0,?(sp) > > ld ra,?(sp) > > add sp,sp,? > > jr t0 // <- back to function entry > > ``` > > > > If we change regs->t0 as direct_caller and execute `jr t0` directly, > > the direct_caller will always jump to itself due to its last `jr` inst. > > > > ``` > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > > function_trace_call // write ringbuffer > > | > > -> call_direct_funcs // regs->t0 = direct_caller > > -> jr t0 // goto direct_caller > > > > direct_caller: > > ... > > sd t0,?(sp) > > ... > > ld t0,?(s0) > > ... > > jr t0 // goto direct_caller always > > ``` > > > > Hope I made it clear. > > > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > > offset, it should only > > > > changed by the IPMODIFY ops instead of the direct_ops. > > > > >> > > > > >> +} > > > > >> + > > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > >> ENTRY(ftrace_regs_caller) > > > > >> SAVE_ABI_REGS 1 > > > > >> + REG_S x0, PT_T1(sp) > > > > >> PREPARE_ARGS > > > > >> > > > > >> ftrace_regs_call: > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > >> > > > > >> > > > > >> RESTORE_ABI_REGS 1 > > > > >> + bnez t1,.Ldirect > > > > >> jr t0 > > > > >> +.Ldirect: > > > > >> + jr t1 > > > > >> ENDPROC(ftrace_regs_caller) > > > > >> > > > > >> ENTRY(ftrace_caller) > > > > >> -- > > > > >> 2.20.1 > > > > >> > > > > > > > > > > > > > > > -- > > > > > Best Regards > > > > > Guo Ren > > > > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > Thanks, > > Song > Thanks, > Song
Guo Ren <guoren@kernel.org> 于2022年11月24日周四 03:40写道: > > On Thu, Nov 24, 2022 at 11:09 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > Song Shuai <suagrfillet@gmail.com> 于2022年11月24日周四 02:52写道: > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 02:08写道: > > > > > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > > > > > Cool job, thx. > > > > > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > >> > > > > > >> 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> > > > > > >> --- > > > > > >> arch/riscv/Kconfig | 1 + > > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > > > >> 3 files changed, 11 insertions(+) > > > > > >> > > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > > > >> --- a/arch/riscv/Kconfig > > > > > >> +++ b/arch/riscv/Kconfig > > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > > >> index 01bebb28eabe..be4d57566139 100644 > > > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > > > > > How about regs->t0 = addr; ? > > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > > > the reg t0 will never be changed when direct_caller returns. > > > > > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > > > direct_caller will enter the dead loop. > > > > ? > > > > > > > > ftrace_regs_caller->call_direct_funcs-> > > > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0. > > > > > > > > Only call_direct_funcs call arch_ftrace_set_direct_caller ! > > > > > > > > static void call_direct_funcs(unsigned long ip, unsigned long pip, > > > > struct ftrace_ops *ops, struct ftrace_regs *fregs) > > > > { > > > > struct pt_regs *regs = ftrace_get_regs(fregs); > > > > unsigned long addr; > > > > > > > > addr = ftrace_find_rec_direct(ip); > > > > if (!addr) > > > > return; > > > > > > > > arch_ftrace_set_direct_caller(regs, addr); > > > > } > > > > > > > When direct_caller and function tracer co-hook a function, the simple > > > diagram is like this: > > > > > > ``` > > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > > > function_trace_call // write ringbuffer > > > | > > > -> call_direct_funcs // regs->t1 = direct_caller > > > -> t1 !=0 && jr t1 // goto direct_caller > > > ``` > > > > > ``` > > f -- regs_caller -- list_func > > | | -- function_trace_call > > | | -- call_direct_funcs // t1 = addr > > |-- t1 !=0 && jr t1 // goto direct_caller > Cool diagram! Thx, but we still need a discussion: > f -- regs_caller > | -- SAVE_ABI_REGS 1 > | -- list_func > | | -- function_trace_call > | | -- call_direct_funcs // t1 = addr > | -- RESTORE_ABI_REGS 1 > |-- t1 !=0 && jr t1 // goto direct_caller > If you set t1 non-zero, then we always go to direct_caller. I think > the code is equal to set t0=addr. If only focusing on the whole ftrace_regs_caller code, you're right. But we should also take direct_caller code into the consideration, if using t0 here, direct_caller will always return to itself as I described before. > | | -- call_direct_funcs // t0 = addr > | -- RESTORE_ABI_REGS 1 > |-- jr t0 // goto direct_caller > > I think the only problem happens in the below non-existent situation: > f -- regs_caller > | -- SAVE_ABI_REGS 1 > | -- list_func > | | -- call_direct_funcs // t0 = addr > | | -- function_trace_call //t0 contains > direct_caller instead function_trace_call is one type of global_ops->func which doesn't take care of the regs->t0. But the func of IPMODIFY ops (eg. livepatch, kprobe with posthandler) will change regs->epc which will then be restored to t0 in ftrace_regs_caller. And then the address of direct_caller will be covered in this situation. I'm not very clear about the process of the livepatch and kprobe in RISCV but I think we should keep t0 for their ipmodifying. > | -- RESTORE_ABI_REGS 1 > |-- jr t0 // goto direct_caller > > The key issue is whether going to direct_caller correctly depends on > call_direct_funcs called, right? Yes, in other words, call_direct_func informs ftrace_regs_caller that there is a direct caller stored in t1, please jump to it first. > > > ``` > > This diagram looks better. > > > And the direct_caller has a similar implement as ftrace_caller. > > > > > > ``` > > > direct_caller: > > > add sp,sp,-? > > > sd t0,?(sp) > > > sd ra,?(sp) > > > call foo > > > ld t0,?(sp) > > > ld ra,?(sp) > > > add sp,sp,? > > > jr t0 // <- back to function entry > > > ``` > > > > > > If we change regs->t0 as direct_caller and execute `jr t0` directly, > > > the direct_caller will always jump to itself due to its last `jr` inst. --- Here is what I described in the previous email. > > > > > > ``` > > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > > > function_trace_call // write ringbuffer > > > | > > > -> call_direct_funcs // regs->t0 = direct_caller > > > -> jr t0 // goto direct_caller > > > > > > direct_caller: > > > ... > > > sd t0,?(sp) > > > ... > > > ld t0,?(s0) > > > ... > > > jr t0 // goto direct_caller always > > > ``` > > > > > > Hope I made it clear. > > > > > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > > > offset, it should only > > > > > changed by the IPMODIFY ops instead of the direct_ops. > > > > > >> > > > > > >> +} > > > > > >> + > > > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > > >> ENTRY(ftrace_regs_caller) > > > > > >> SAVE_ABI_REGS 1 > > > > > >> + REG_S x0, PT_T1(sp) > > > > > >> PREPARE_ARGS > > > > > >> > > > > > >> ftrace_regs_call: > > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > > >> > > > > > >> > > > > > >> RESTORE_ABI_REGS 1 > > > > > >> + bnez t1,.Ldirect > > > > > >> jr t0 > > > > > >> +.Ldirect: > > > > > >> + jr t1 > > > > > >> ENDPROC(ftrace_regs_caller) > > > > > >> > > > > > >> ENTRY(ftrace_caller) > > > > > >> -- > > > > > >> 2.20.1 > > > > > >> > > > > > > > > > > > > > > > > > > -- > > > > > > Best Regards > > > > > > Guo Ren > > > > > > > > > > > > > > > > -- > > > > Best Regards > > > > Guo Ren > > > Thanks, > > > Song > > Thanks, > > Song > > > > -- > Best Regards > Guo Ren Thanks, Song
On Thu, Nov 24, 2022 at 7:49 PM Song Shuai <suagrfillet@gmail.com> wrote: > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 03:40写道: > > > > On Thu, Nov 24, 2022 at 11:09 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > Song Shuai <suagrfillet@gmail.com> 于2022年11月24日周四 02:52写道: > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 02:08写道: > > > > > > > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > > > > > > > Cool job, thx. > > > > > > > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > >> > > > > > > >> 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> > > > > > > >> --- > > > > > > >> arch/riscv/Kconfig | 1 + > > > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > > > > >> 3 files changed, 11 insertions(+) > > > > > > >> > > > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > > > > >> --- a/arch/riscv/Kconfig > > > > > > >> +++ b/arch/riscv/Kconfig > > > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > > > >> index 01bebb28eabe..be4d57566139 100644 > > > > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > > > > > > > How about regs->t0 = addr; ? > > > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > > > > the reg t0 will never be changed when direct_caller returns. > > > > > > > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > > > > direct_caller will enter the dead loop. > > > > > ? > > > > > > > > > > ftrace_regs_caller->call_direct_funcs-> > > > > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0. > > > > > > > > > > Only call_direct_funcs call arch_ftrace_set_direct_caller ! > > > > > > > > > > static void call_direct_funcs(unsigned long ip, unsigned long pip, > > > > > struct ftrace_ops *ops, struct ftrace_regs *fregs) > > > > > { > > > > > struct pt_regs *regs = ftrace_get_regs(fregs); > > > > > unsigned long addr; > > > > > > > > > > addr = ftrace_find_rec_direct(ip); > > > > > if (!addr) > > > > > return; > > > > > > > > > > arch_ftrace_set_direct_caller(regs, addr); > > > > > } > > > > > > > > > When direct_caller and function tracer co-hook a function, the simple > > > > diagram is like this: > > > > > > > > ``` > > > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > > > > function_trace_call // write ringbuffer > > > > | > > > > -> call_direct_funcs // regs->t1 = direct_caller > > > > -> t1 !=0 && jr t1 // goto direct_caller > > > > ``` > > > > > > > ``` > > > f -- regs_caller -- list_func > > > | | -- function_trace_call > > > | | -- call_direct_funcs // t1 = addr > > > |-- t1 !=0 && jr t1 // goto direct_caller > > Cool diagram! Thx, but we still need a discussion: > > f -- regs_caller > > | -- SAVE_ABI_REGS 1 > > | -- list_func > > | | -- function_trace_call > > | | -- call_direct_funcs // t1 = addr > > | -- RESTORE_ABI_REGS 1 > > |-- t1 !=0 && jr t1 // goto direct_caller > > If you set t1 non-zero, then we always go to direct_caller. I think > > the code is equal to set t0=addr. > If only focusing on the whole ftrace_regs_caller code, you're right. Yes, that's the problem I have; I didn't look at the sample code. t0 -> ftrace caller t1 -> my_tramp1 (direct caller) ra -> original func return addr. Okay, I would include these patches in v4. Reviewed-by: Guo Ren <guoren@kernel.org> > > But we should also take direct_caller code into the consideration, > if using t0 here, direct_caller will always return to itself as I > described before. > > | | -- call_direct_funcs // t0 = addr > > | -- RESTORE_ABI_REGS 1 > > |-- jr t0 // goto direct_caller > > > > I think the only problem happens in the below non-existent situation: > > f -- regs_caller > > | -- SAVE_ABI_REGS 1 > > | -- list_func > > | | -- call_direct_funcs // t0 = addr > > | | -- function_trace_call //t0 contains > > direct_caller instead > function_trace_call is one type of global_ops->func which doesn't take care > of the regs->t0. > > But the func of IPMODIFY ops (eg. livepatch, kprobe with posthandler) > will change regs->epc which will then be restored to t0 in ftrace_regs_caller. > And then the address of direct_caller will be covered in this situation. > I'm not very clear about the process of the livepatch and kprobe in RISCV > but I think we should keep t0 for their ipmodifying. > > | -- RESTORE_ABI_REGS 1 > > |-- jr t0 // goto direct_caller > > > > The key issue is whether going to direct_caller correctly depends on > > call_direct_funcs called, right? > Yes, in other words, call_direct_func informs ftrace_regs_caller that > there is a direct caller stored in t1, please jump to it first. > > > > > ``` > > > This diagram looks better. > > > > And the direct_caller has a similar implement as ftrace_caller. > > > > > > > > ``` > > > > direct_caller: > > > > add sp,sp,-? > > > > sd t0,?(sp) > > > > sd ra,?(sp) > > > > call foo > > > > ld t0,?(sp) > > > > ld ra,?(sp) > > > > add sp,sp,? > > > > jr t0 // <- back to function entry > > > > ``` > > > > > > > > If we change regs->t0 as direct_caller and execute `jr t0` directly, > > > > the direct_caller will always jump to itself due to its last `jr` inst. > --- Here is what I described in the previous email. > > > > > > > > ``` > > > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func -> > > > > function_trace_call // write ringbuffer > > > > | > > > > -> call_direct_funcs // regs->t0 = direct_caller > > > > -> jr t0 // goto direct_caller > > > > > > > > direct_caller: > > > > ... > > > > sd t0,?(sp) > > > > ... > > > > ld t0,?(s0) > > > > ... > > > > jr t0 // goto direct_caller always > > > > ``` > > > > > > > > Hope I made it clear. > > > > > > > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > > > > offset, it should only > > > > > > changed by the IPMODIFY ops instead of the direct_ops. > > > > > > >> > > > > > > >> +} > > > > > > >> + > > > > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > > > >> ENTRY(ftrace_regs_caller) > > > > > > >> SAVE_ABI_REGS 1 > > > > > > >> + REG_S x0, PT_T1(sp) > > > > > > >> PREPARE_ARGS > > > > > > >> > > > > > > >> ftrace_regs_call: > > > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > > > >> > > > > > > >> > > > > > > >> RESTORE_ABI_REGS 1 > > > > > > >> + bnez t1,.Ldirect > > > > > > >> jr t0 > > > > > > >> +.Ldirect: > > > > > > >> + jr t1 > > > > > > >> ENDPROC(ftrace_regs_caller) > > > > > > >> > > > > > > >> ENTRY(ftrace_caller) > > > > > > >> -- > > > > > > >> 2.20.1 > > > > > > >> > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best Regards > > > > > > > Guo Ren > > > > > > > > > > > > > > > > > > > > -- > > > > > Best Regards > > > > > Guo Ren > > > > Thanks, > > > > Song > > > Thanks, > > > Song > > > > > > > > -- > > Best Regards > > Guo Ren > Thanks, > Song
On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > Cool job, thx. > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > >> > >> 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> > >> --- > >> arch/riscv/Kconfig | 1 + > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > >> 3 files changed, 11 insertions(+) > >> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > >> index 39ec8d628cf6..d083ec08d0b6 100644 > >> --- a/arch/riscv/Kconfig > >> +++ b/arch/riscv/Kconfig > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > >> select HAVE_FUNCTION_GRAPH_TRACER > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > >> index 01bebb28eabe..be4d57566139 100644 > >> --- a/arch/riscv/include/asm/ftrace.h > >> +++ b/arch/riscv/include/asm/ftrace.h > >> @@ -114,6 +114,12 @@ 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; > > > > How about regs->t0 = addr; ? > > And delete all mcount-dyn.S modification. > > > The direct_caller has the same program layout as the ftrace_caller, which means > the reg t0 will never be changed when direct_caller returns. > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > direct_caller will enter the dead loop. > > Actually the reg t0 always saves the address of function entry with 8B > offset, it should only > changed by the IPMODIFY ops instead of the direct_ops. How about: static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) { regs->t1 = regs->t0; regs->t0 = addr; direct_caller: add sp,sp,-? sd t1,?(sp) sd ra,?(sp) call foo ld t1,?(sp) ld ra,?(sp) add sp,sp,? jr t1 // <- back to function entry And delete all mcount-dyn.S modification. > >> > >> +} > >> + > >> #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 466c6ef217b1..b89c85a58569 100644 > >> --- a/arch/riscv/kernel/mcount-dyn.S > >> +++ b/arch/riscv/kernel/mcount-dyn.S > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > >> ENTRY(ftrace_regs_caller) > >> SAVE_ABI_REGS 1 > >> + REG_S x0, PT_T1(sp) > >> PREPARE_ARGS > >> > >> ftrace_regs_call: > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > >> > >> > >> RESTORE_ABI_REGS 1 > >> + bnez t1,.Ldirect > >> jr t0 > >> +.Ldirect: > >> + jr t1 > >> ENDPROC(ftrace_regs_caller) > >> > >> ENTRY(ftrace_caller) > >> -- > >> 2.20.1 > >> > > > > > > -- > > Best Regards > > Guo Ren
Guo Ren <guoren@kernel.org> 于2022年11月24日周四 15:31写道: > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > Cool job, thx. > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > >> > > >> 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> > > >> --- > > >> arch/riscv/Kconfig | 1 + > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > >> 3 files changed, 11 insertions(+) > > >> > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > >> --- a/arch/riscv/Kconfig > > >> +++ b/arch/riscv/Kconfig > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > >> select HAVE_FUNCTION_GRAPH_TRACER > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > >> index 01bebb28eabe..be4d57566139 100644 > > >> --- a/arch/riscv/include/asm/ftrace.h > > >> +++ b/arch/riscv/include/asm/ftrace.h > > >> @@ -114,6 +114,12 @@ 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; > > > > > > How about regs->t0 = addr; ? > > > And delete all mcount-dyn.S modification. > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > the reg t0 will never be changed when direct_caller returns. > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > direct_caller will enter the dead loop. > > > > Actually the reg t0 always saves the address of function entry with 8B > > offset, it should only > > changed by the IPMODIFY ops instead of the direct_ops. > How about: > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, > unsigned long addr) > { > regs->t1 = regs->t0; > regs->t0 = addr; > > direct_caller: > add sp,sp,-? > sd t1,?(sp) direct_caller also serves as the first trampoline as ftrace_caller, like this: ``` func -- direct_caller -- ftrace_[regs]_caller ``` So the t1 in this line has to be t0 to save the PC. > sd ra,?(sp) > call foo > ld t1,?(sp) And this line. > ld ra,?(sp) > add sp,sp,? > jr t1 // <- back to function entry > > And delete all mcount-dyn.S modification. > > > >> > > >> +} > > >> + > > >> #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 466c6ef217b1..b89c85a58569 100644 > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > >> ENTRY(ftrace_regs_caller) > > >> SAVE_ABI_REGS 1 > > >> + REG_S x0, PT_T1(sp) > > >> PREPARE_ARGS > > >> > > >> ftrace_regs_call: > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > >> > > >> > > >> RESTORE_ABI_REGS 1 > > >> + bnez t1,.Ldirect > > >> jr t0 > > >> +.Ldirect: > > >> + jr t1 > > >> ENDPROC(ftrace_regs_caller) > > >> > > >> ENTRY(ftrace_caller) > > >> -- > > >> 2.20.1 > > >> > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > > > -- > Best Regards > Guo Ren Thanks, Song
On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <suagrfillet@gmail.com> wrote: > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 15:31写道: > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > Cool job, thx. > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > >> > > > >> 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> > > > >> --- > > > >> arch/riscv/Kconfig | 1 + > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > >> 3 files changed, 11 insertions(+) > > > >> > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > >> --- a/arch/riscv/Kconfig > > > >> +++ b/arch/riscv/Kconfig > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > >> index 01bebb28eabe..be4d57566139 100644 > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > How about regs->t0 = addr; ? > > > > And delete all mcount-dyn.S modification. > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > the reg t0 will never be changed when direct_caller returns. > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > direct_caller will enter the dead loop. > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > offset, it should only > > > changed by the IPMODIFY ops instead of the direct_ops. > > How about: > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, > > unsigned long addr) > > { > > regs->t1 = regs->t0; > > regs->t0 = addr; > > > > direct_caller: > > add sp,sp,-? > > sd t1,?(sp) > direct_caller also serves as the first trampoline as ftrace_caller, like this: > ``` > func -- direct_caller > -- ftrace_[regs]_caller > ``` > So the t1 in this line has to be t0 to save the PC. direct_caller: add sp,sp,-? sd t1,?(sp) sd t0, ?(so) sd ra,?(sp) mov t0, t1 call foo ld t0,?(sp) ld t1,?(sp) ld ra,?(sp) add sp,sp,? jr t1 // <- back to function entry > > sd ra,?(sp) > > call foo > > ld t1,?(sp) > And this line. > > ld ra,?(sp) > > add sp,sp,? > > jr t1 // <- back to function entry > > > > And delete all mcount-dyn.S modification. > > > > > >> > > > >> +} > > > >> + > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > >> ENTRY(ftrace_regs_caller) > > > >> SAVE_ABI_REGS 1 > > > >> + REG_S x0, PT_T1(sp) > > > >> PREPARE_ARGS > > > >> > > > >> ftrace_regs_call: > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > >> > > > >> > > > >> RESTORE_ABI_REGS 1 > > > >> + bnez t1,.Ldirect > > > >> jr t0 > > > >> +.Ldirect: > > > >> + jr t1 > > > >> ENDPROC(ftrace_regs_caller) > > > >> > > > >> ENTRY(ftrace_caller) > > > >> -- > > > >> 2.20.1 > > > >> > > > > > > > > > > > > -- > > > > Best Regards > > > > Guo Ren > > > > > > > > -- > > Best Regards > > Guo Ren > Thanks, > Song
Guo Ren <guoren@kernel.org> 于2022年11月25日周五 03:10写道: > > On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 15:31写道: > > > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > > > Cool job, thx. > > > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > >> > > > > >> 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> > > > > >> --- > > > > >> arch/riscv/Kconfig | 1 + > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > > >> 3 files changed, 11 insertions(+) > > > > >> > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > > >> --- a/arch/riscv/Kconfig > > > > >> +++ b/arch/riscv/Kconfig > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > >> index 01bebb28eabe..be4d57566139 100644 > > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > > > How about regs->t0 = addr; ? > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > > the reg t0 will never be changed when direct_caller returns. > > > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > > direct_caller will enter the dead loop. > > > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > > offset, it should only > > > > changed by the IPMODIFY ops instead of the direct_ops. > > > How about: > > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, > > > unsigned long addr) > > > { > > > regs->t1 = regs->t0; > > > regs->t0 = addr; > > > > > > direct_caller: > > > add sp,sp,-? > > > sd t1,?(sp) > > direct_caller also serves as the first trampoline as ftrace_caller, like this: > > ``` > > func -- direct_caller > > -- ftrace_[regs]_caller > > ``` > > So the t1 in this line has to be t0 to save the PC. > > direct_caller: > add sp,sp,-? > sd t1,?(sp) > sd t0, ?(so) > sd ra,?(sp) > mov t0, t1 This foo is the tracing function along with the direct_caller, and it has the same parameters as the target function. So the t0 or t1 here means nothing for this foo function. No offense, but what's the purpose of this mv inst? > call foo > ld t0,?(sp) > ld t1,?(sp) > ld ra,?(sp) > add sp,sp,? > jr t1 // <- back to function entry When direct_caller works as the first trampoline the content of t1 here means nothing for the target function, neither PC nor PIP. > > > > > sd ra,?(sp) > > > call foo > > > ld t1,?(sp) > > And this line. > > > ld ra,?(sp) > > > add sp,sp,? > > > jr t1 // <- back to function entry > > > > > > And delete all mcount-dyn.S modification. > > > > > > > >> > > > > >> +} > > > > >> + > > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > >> ENTRY(ftrace_regs_caller) > > > > >> SAVE_ABI_REGS 1 > > > > >> + REG_S x0, PT_T1(sp) > > > > >> PREPARE_ARGS > > > > >> > > > > >> ftrace_regs_call: > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > >> > > > > >> > > > > >> RESTORE_ABI_REGS 1 > > > > >> + bnez t1,.Ldirect > > > > >> jr t0 > > > > >> +.Ldirect: > > > > >> + jr t1 > > > > >> ENDPROC(ftrace_regs_caller) > > > > >> > > > > >> ENTRY(ftrace_caller) > > > > >> -- > > > > >> 2.20.1 > > > > >> > > > > > > > > > > > > > > > -- > > > > > Best Regards > > > > > Guo Ren > > > > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > Thanks, > > Song > > > > -- > Best Regards > Guo Ren
On Fri, Nov 25, 2022 at 2:33 PM Song Shuai <suagrfillet@gmail.com> wrote: > > Guo Ren <guoren@kernel.org> 于2022年11月25日周五 03:10写道: > > > > On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 15:31写道: > > > > > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > > > > > Cool job, thx. > > > > > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > >> > > > > > >> 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> > > > > > >> --- > > > > > >> arch/riscv/Kconfig | 1 + > > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > > > >> 3 files changed, 11 insertions(+) > > > > > >> > > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > > > >> --- a/arch/riscv/Kconfig > > > > > >> +++ b/arch/riscv/Kconfig > > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > > >> index 01bebb28eabe..be4d57566139 100644 > > > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > > > > > How about regs->t0 = addr; ? > > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > > > the reg t0 will never be changed when direct_caller returns. > > > > > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > > > direct_caller will enter the dead loop. > > > > > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > > > offset, it should only > > > > > changed by the IPMODIFY ops instead of the direct_ops. > > > > How about: > > > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, > > > > unsigned long addr) > > > > { > > > > regs->t1 = regs->t0; > > > > regs->t0 = addr; > > > > > > > > direct_caller: > > > > add sp,sp,-? > > > > sd t1,?(sp) > > > direct_caller also serves as the first trampoline as ftrace_caller, like this: > > > ``` > > > func -- direct_caller > > > -- ftrace_[regs]_caller > > > ``` > > > So the t1 in this line has to be t0 to save the PC. > > > > direct_caller: > > add sp,sp,-? > > sd t1,?(sp) > > sd t0, ?(so) > > sd ra,?(sp) > > mov t0, t1 > This foo is the tracing function along with the direct_caller, > and it has the same parameters as the target function. > So the t0 or t1 here means nothing for this foo function. > > No offense, but what's the purpose of this mv inst? Here is my modification: [1] [1]: https://lore.kernel.org/linux-riscv/20221128101201.4144527-1-guoren@kernel.org/ I've tested like this: mount -t debugfs none /sys/kernel/debug/ cd /sys/kernel/debug/tracing/ echo handle_mm_fault > set_ftrace_filter echo 'r:myr handle_mm_fault' > kprobe_events echo function > current_tracer echo 1 > events/kprobes/myr/enable insmod /root/ftrace/ftrace-direct-too.ko cat trace cat-137 [000] ..... 3132.231948: handle_mm_fault <-do_page_fault cat-137 [000] ..... 3132.231973: my_direct_func: handle mm fault vma=000000001ba58b17 address=aaaaaaaab689c8 flags=254 cat-137 [000] ..... 3132.232622: myr: (do_page_fault+0x176/0x424 <- handle_mm_fault) cat-137 [000] ..... 3132.232724: handle_mm_fault <-do_page_fault cat-137 [000] ..... 3132.232750: my_direct_func: handle mm fault vma=000000001ba58b17 address=aaaaaaaab7df9c flags=254 cat-137 [000] ..... 3132.233385: myr: (do_page_fault+0x176/0x424 <- handle_mm_fault) cat-137 [000] ..... 3132.233744: handle_mm_fault <-do_page_fault cat-137 [000] ..... 3132.233772: my_direct_func: handle mm fault vma=000000001ba58b17 address=aaaaaaaab2b2c8 flags=354 cat-137 [000] ..... 3132.234392: myr: (do_page_fault+0x176/0x424 <- handle_mm_fault) cat-137 [000] ..... 3132.234480: handle_mm_fault <-do_page_fault cat-137 [000] ..... 3132.234505: my_direct_func: handle mm fault vma=000000001ba58b17 address=aaaaaaaab5afc0 flags=354 cat-137 [000] ..... 3132.235149: myr: (do_page_fault+0x176/0x424 <- handle_mm_fault) cat-137 [000] ..... 3132.235263: handle_mm_fault <-do_page_fault cat-137 [000] ..... 3132.235289: my_direct_func: handle mm fault vma=0000000071a096de address=fffffff7fac530 flags=254 cat-137 [000] ..... 3132.235893: myr: (do_page_fault+0x176/0x424 <- handle_mm_fault) > > call foo > > ld t0,?(sp) > > ld t1,?(sp) > > ld ra,?(sp) > > add sp,sp,? > > jr t1 // <- back to function entry > When direct_caller works as the first trampoline > the content of t1 here means nothing for the target function, neither > PC nor PIP. > > > > > > > > sd ra,?(sp) > > > > call foo > > > > ld t1,?(sp) > > > And this line. > > > > ld ra,?(sp) > > > > add sp,sp,? > > > > jr t1 // <- back to function entry > > > > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > >> > > > > > >> +} > > > > > >> + > > > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > > >> ENTRY(ftrace_regs_caller) > > > > > >> SAVE_ABI_REGS 1 > > > > > >> + REG_S x0, PT_T1(sp) > > > > > >> PREPARE_ARGS > > > > > >> > > > > > >> ftrace_regs_call: > > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > > >> > > > > > >> > > > > > >> RESTORE_ABI_REGS 1 > > > > > >> + bnez t1,.Ldirect > > > > > >> jr t0 > > > > > >> +.Ldirect: > > > > > >> + jr t1 > > > > > >> ENDPROC(ftrace_regs_caller) > > > > > >> > > > > > >> ENTRY(ftrace_caller) > > > > > >> -- > > > > > >> 2.20.1 > > > > > >> > > > > > > > > > > > > > > > > > > -- > > > > > > Best Regards > > > > > > Guo Ren > > > > > > > > > > > > > > > > -- > > > > Best Regards > > > > Guo Ren > > > Thanks, > > > Song > > > > > > > > -- > > Best Regards > > Guo Ren
On Mon, Nov 28, 2022 at 6:17 PM Guo Ren <guoren@kernel.org> wrote: > > On Fri, Nov 25, 2022 at 2:33 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > Guo Ren <guoren@kernel.org> 于2022年11月25日周五 03:10写道: > > > > > > On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月24日周四 15:31写道: > > > > > > > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > > > > > > > Guo Ren <guoren@kernel.org> 于2022年11月23日周三 23:02写道: > > > > > > > > > > > > > > Cool job, thx. > > > > > > > > > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > > > >> > > > > > > >> 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> > > > > > > >> --- > > > > > > >> arch/riscv/Kconfig | 1 + > > > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++ > > > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > > > > > >> 3 files changed, 11 insertions(+) > > > > > > >> > > > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644 > > > > > > >> --- a/arch/riscv/Kconfig > > > > > > >> +++ b/arch/riscv/Kconfig > > > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I > > > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > > > > > >> select HAVE_FUNCTION_GRAPH_TRACER > > > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > > > > > >> index 01bebb28eabe..be4d57566139 100644 > > > > > > >> --- a/arch/riscv/include/asm/ftrace.h > > > > > > >> +++ b/arch/riscv/include/asm/ftrace.h > > > > > > >> @@ -114,6 +114,12 @@ 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; > > > > > > > > > > > > > > How about regs->t0 = addr; ? > > > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > > > > > The direct_caller has the same program layout as the ftrace_caller, which means > > > > > > the reg t0 will never be changed when direct_caller returns. > > > > > > > > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`, > > > > > > direct_caller will enter the dead loop. > > > > > > > > > > > > Actually the reg t0 always saves the address of function entry with 8B > > > > > > offset, it should only > > > > > > changed by the IPMODIFY ops instead of the direct_ops. > > > > > How about: > > > > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, > > > > > unsigned long addr) > > > > > { > > > > > regs->t1 = regs->t0; > > > > > regs->t0 = addr; > > > > > > > > > > direct_caller: > > > > > add sp,sp,-? > > > > > sd t1,?(sp) > > > > direct_caller also serves as the first trampoline as ftrace_caller, like this: > > > > ``` > > > > func -- direct_caller > > > > -- ftrace_[regs]_caller > > > > ``` > > > > So the t1 in this line has to be t0 to save the PC. > > > > > > direct_caller: > > > add sp,sp,-? > > > sd t1,?(sp) > > > sd t0, ?(so) > > > sd ra,?(sp) > > > mov t0, t1 > > This foo is the tracing function along with the direct_caller, > > and it has the same parameters as the target function. > > So the t0 or t1 here means nothing for this foo function. > > > > No offense, but what's the purpose of this mv inst? > Here is my modification: [1] > [1]: https://lore.kernel.org/linux-riscv/20221128101201.4144527-1-guoren@kernel.org/ > > I've tested like this: > mount -t debugfs none /sys/kernel/debug/ > cd /sys/kernel/debug/tracing/ > echo handle_mm_fault > set_ftrace_filter > echo 'r:myr handle_mm_fault' > kprobe_events > echo function > current_tracer > echo 1 > events/kprobes/myr/enable > insmod /root/ftrace/ftrace-direct-too.ko > cat trace > > cat-137 [000] ..... 3132.231948: handle_mm_fault > <-do_page_fault > cat-137 [000] ..... 3132.231973: my_direct_func: > handle mm fault vma=000000001ba58b17 address=aaaaaaaab689c8 flags=254 > cat-137 [000] ..... 3132.232622: myr: > (do_page_fault+0x176/0x424 <- handle_mm_fault) > cat-137 [000] ..... 3132.232724: handle_mm_fault > <-do_page_fault > cat-137 [000] ..... 3132.232750: my_direct_func: > handle mm fault vma=000000001ba58b17 address=aaaaaaaab7df9c flags=254 > cat-137 [000] ..... 3132.233385: myr: > (do_page_fault+0x176/0x424 <- handle_mm_fault) > cat-137 [000] ..... 3132.233744: handle_mm_fault > <-do_page_fault > cat-137 [000] ..... 3132.233772: my_direct_func: > handle mm fault vma=000000001ba58b17 address=aaaaaaaab2b2c8 flags=354 > cat-137 [000] ..... 3132.234392: myr: > (do_page_fault+0x176/0x424 <- handle_mm_fault) > cat-137 [000] ..... 3132.234480: handle_mm_fault > <-do_page_fault > cat-137 [000] ..... 3132.234505: my_direct_func: > handle mm fault vma=000000001ba58b17 address=aaaaaaaab5afc0 flags=354 > cat-137 [000] ..... 3132.235149: myr: > (do_page_fault+0x176/0x424 <- handle_mm_fault) > cat-137 [000] ..... 3132.235263: handle_mm_fault > <-do_page_fault > cat-137 [000] ..... 3132.235289: my_direct_func: > handle mm fault vma=0000000071a096de address=fffffff7fac530 flags=254 > cat-137 [000] ..... 3132.235893: myr: > (do_page_fault+0x176/0x424 <- handle_mm_fault) I got a fail with no ftrace enabled. Yes, your patch is correct. # insmod /root/ftrace/ftrace-direct-too.ko [ 13.586821] [ 13.586970] ********************************************************** [ 13.587186] ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE ** [ 13.587396] ** ** [ 13.587594] ** trace_printk() being used. Allocating extra memory. ** [ 13.587804] ** ** [ 13.588240] ** This means that this is a DEBUG kernel and it is ** [ 13.588488] ** unsafe for production use. ** [ 13.588735] ** ** [ 13.588966] ** If you see this message and you are not debugging ** [ 13.589200] ** the kernel, report this immediately to your vendor! ** [ 13.589435] ** ** [ 13.589682] ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE ** [ 13.589922] ********************************************************** [ 13.632876] Unable to handle kernel access to user memory without uaccess routines at virtual address 0000000000000000 [ 13.633717] Oops [#1] [ 13.633858] Modules linked in: ftrace_direct_too [ 13.634294] CPU: 0 PID: 121 Comm: sh Not tainted 6.1.0-rc1-00016-g459bb40f7d97 #388 [ 13.634652] Hardware name: riscv-virtio,qemu (DT) [ 13.634962] epc : 0x0 [ 13.635313] ra : do_page_fault+0x176/0x424 [ 13.635548] epc : 0000000000000000 ra : ffffffff8000c8d0 sp : ff2000000083be80 [ 13.635819] gp : ffffffff815dc778 tp : ff600000033a9680 t0 : 0000000000000000 [ 13.636144] t1 : 0000000000000000 t2 : 0000000000000000 s0 : ff2000000083bee0 [ 13.636427] s1 : ff2000000083bee0 a0 : ff600000038cb660 a1 : 00fffffff7e74790 [ 13.636725] a2 : 0000000000000255 a3 : 0000000000000000 a4 : 0000000000000000 [ 13.637022] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 [ 13.637308] s2 : 00fffffff7e74790 s3 : 000000000000000f s4 : ff6000000319b400 [ 13.637600] s5 : ff600000033a9680 s6 : ff600000038cb660 s7 : ff6000000319b460 [ 13.637889] s8 : 0000000000000255 s9 : 0000000000000001 s10: 00fffffffffff4dc [ 13.638172] s11: 0000000000000004 t3 : 0000000000000000 t4 : 0000000000000000 [ 13.638453] t5 : 0000000000000000 t6 : 0000000000000000 [ 13.638679] status: 0000000200000120 badaddr: 0000000000000000 cause: 000000000000000c [ 13.640035] ---[ end trace 0000000000000000 ]--- > > > > > call foo > > > ld t0,?(sp) > > > ld t1,?(sp) > > > ld ra,?(sp) > > > add sp,sp,? > > > jr t1 // <- back to function entry > > When direct_caller works as the first trampoline > > the content of t1 here means nothing for the target function, neither > > PC nor PIP. > > > > > > > > > > > sd ra,?(sp) > > > > > call foo > > > > > ld t1,?(sp) > > > > And this line. > > > > > ld ra,?(sp) > > > > > add sp,sp,? > > > > > jr t1 // <- back to function entry > > > > > > > > > > And delete all mcount-dyn.S modification. > > > > > > > > > > > >> > > > > > > >> +} > > > > > > >> + > > > > > > >> #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 466c6ef217b1..b89c85a58569 100644 > > > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S > > > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S > > > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > > > > > >> ENTRY(ftrace_regs_caller) > > > > > > >> SAVE_ABI_REGS 1 > > > > > > >> + REG_S x0, PT_T1(sp) > > > > > > >> PREPARE_ARGS > > > > > > >> > > > > > > >> ftrace_regs_call: > > > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > > > >> > > > > > > >> > > > > > > >> RESTORE_ABI_REGS 1 > > > > > > >> + bnez t1,.Ldirect > > > > > > >> jr t0 > > > > > > >> +.Ldirect: > > > > > > >> + jr t1 > > > > > > >> ENDPROC(ftrace_regs_caller) > > > > > > >> > > > > > > >> ENTRY(ftrace_caller) > > > > > > >> -- > > > > > > >> 2.20.1 > > > > > > >> > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best Regards > > > > > > > Guo Ren > > > > > > > > > > > > > > > > > > > > -- > > > > > Best Regards > > > > > Guo Ren > > > > Thanks, > > > > Song > > > > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > > > -- > Best Regards > Guo Ren
On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > 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> > --- > arch/riscv/Kconfig | 1 + > arch/riscv/include/asm/ftrace.h | 6 ++++++ > arch/riscv/kernel/mcount-dyn.S | 4 ++++ > 3 files changed, 11 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 39ec8d628cf6..d083ec08d0b6 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -278,6 +278,7 @@ config ARCH_RV64I > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 01bebb28eabe..be4d57566139 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -114,6 +114,12 @@ 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; > +} > + > #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 466c6ef217b1..b89c85a58569 100644 > --- a/arch/riscv/kernel/mcount-dyn.S > +++ b/arch/riscv/kernel/mcount-dyn.S > @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > ENTRY(ftrace_regs_caller) > SAVE_ABI_REGS 1 > + REG_S x0, PT_T1(sp) ENTRY(ftrace_regs_caller) + move t1, zero SAVE_ABI_REGS 1 PREPARE_ARGS Shall we use a move here, instead? > PREPARE_ARGS > > ftrace_regs_call: > @@ -241,7 +242,10 @@ ftrace_regs_call: > > > RESTORE_ABI_REGS 1 > + bnez t1,.Ldirect > jr t0 > +.Ldirect: > + jr t1 > ENDPROC(ftrace_regs_caller) > > ENTRY(ftrace_caller) > -- > 2.20.1 >
Guo Ren <guoren@kernel.org> 于2022年11月28日周一 13:17写道: > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <suagrfillet@gmail.com> wrote: > > > > 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> > > --- > > arch/riscv/Kconfig | 1 + > > arch/riscv/include/asm/ftrace.h | 6 ++++++ > > arch/riscv/kernel/mcount-dyn.S | 4 ++++ > > 3 files changed, 11 insertions(+) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 39ec8d628cf6..d083ec08d0b6 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -278,6 +278,7 @@ config ARCH_RV64I > > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 > > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) > > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > > + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > > select HAVE_FUNCTION_GRAPH_TRACER > > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > > index 01bebb28eabe..be4d57566139 100644 > > --- a/arch/riscv/include/asm/ftrace.h > > +++ b/arch/riscv/include/asm/ftrace.h > > @@ -114,6 +114,12 @@ 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; > > +} > > + > > #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 466c6ef217b1..b89c85a58569 100644 > > --- a/arch/riscv/kernel/mcount-dyn.S > > +++ b/arch/riscv/kernel/mcount-dyn.S > > @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) > > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > ENTRY(ftrace_regs_caller) > > SAVE_ABI_REGS 1 > > + REG_S x0, PT_T1(sp) > ENTRY(ftrace_regs_caller) > + move t1, zero > SAVE_ABI_REGS 1 > PREPARE_ARGS > > Shall we use a move here, instead? That seems ok to me. Just move it. > > > PREPARE_ARGS > > > > ftrace_regs_call: > > @@ -241,7 +242,10 @@ ftrace_regs_call: > > > > > > RESTORE_ABI_REGS 1 > > + bnez t1,.Ldirect > > jr t0 > > +.Ldirect: > > + jr t1 > > ENDPROC(ftrace_regs_caller) > > > > ENTRY(ftrace_caller) > > -- > > 2.20.1 > > > > > -- > Best Regards > Guo Ren
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 39ec8d628cf6..d083ec08d0b6 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -278,6 +278,7 @@ config ARCH_RV64I select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 01bebb28eabe..be4d57566139 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -114,6 +114,12 @@ 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; +} + #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 466c6ef217b1..b89c85a58569 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller) #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_regs_caller) SAVE_ABI_REGS 1 + REG_S x0, PT_T1(sp) PREPARE_ARGS ftrace_regs_call: @@ -241,7 +242,10 @@ ftrace_regs_call: RESTORE_ABI_REGS 1 + bnez t1,.Ldirect jr t0 +.Ldirect: + jr t1 ENDPROC(ftrace_regs_caller) ENTRY(ftrace_caller)