From: Masami Hiramatsu <mhiramat@kernel.org>
To: guoren@kernel.org
Cc: palmerdabbelt@google.com, paul.walmsley@sifive.com,
anup@brainfault.org, greentime.hu@sifive.com, zong.li@sifive.com,
me@packi.ch, bjorn.topel@gmail.com, atish.patra@wdc.com,
penberg@kernel.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-csky@vger.kernel.org,
Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported
Date: Fri, 10 Jul 2020 22:50:17 +0900 [thread overview]
Message-ID: <20200710225017.5ce329485e911f99e17cd483@kernel.org> (raw)
In-Reply-To: <1594261154-69745-7-git-send-email-guoren@kernel.org>
Hi Guo,
On Thu, 9 Jul 2020 02:19:14 +0000
guoren@kernel.org wrote:
> +/* Ftrace callback handler for kprobes -- called under preepmt disabed */
> +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> + struct kprobe *p;
> + struct kprobe_ctlblk *kcb;
> +
> + p = get_kprobe((kprobe_opcode_t *)ip);
> + if (unlikely(!p) || kprobe_disabled(p))
> + return;
> +
> + kcb = get_kprobe_ctlblk();
> + if (kprobe_running()) {
> + kprobes_inc_nmissed_count(p);
> + } else {
> + /*
> + * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
> + * So no need to resume it, just for kprobe handler.
> + */
> + instruction_pointer_set(regs, ip);
> + __this_cpu_write(current_kprobe, p);
> + kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> + if (!p->pre_handler || !p->pre_handler(p, regs)) {
> + /*
> + * Emulate singlestep (and also recover regs->pc)
> + * as if there is a nop
> + */
> + instruction_pointer_set(regs,
> + (unsigned long)p->addr + MCOUNT_INSN_SIZE);
> + if (unlikely(p->post_handler)) {
> + kcb->kprobe_status = KPROBE_HIT_SSDONE;
> + p->post_handler(p, regs, 0);
> + }
Hmm, don't you need restoring the previous instruction pointer here?
If you don't support modifying the instruction pointer in the handler,
it must not be compatible with kprobes.
Now BPF function override and function error injection depends on
this behevior, so could you consider to support it in the "ftrace"
implementation at first? (And if it is enabled, you can enable the
livepatch on RISCV too)
Thank you,
> + }
> +
> + /*
> + * If pre_handler returns !0, it changes regs->pc. We have to
> + * skip emulating post_handler.
> + */
> + __this_cpu_write(current_kprobe, NULL);
> + }
> +}
> +NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> +
> +int arch_prepare_kprobe_ftrace(struct kprobe *p)
> +{
> + p->ainsn.api.insn = NULL;
> + return 0;
> +}
> --
> 2.7.4
>
--
Masami Hiramatsu <mhiramat@kernel.org>
next prev parent reply other threads:[~2020-07-10 13:50 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-09 2:19 [PATCH v2 0/6] riscv: Add k/uprobe supported guoren
2020-07-09 2:19 ` [PATCH v2 1/6] riscv: Fixup __vdso_gettimeofday broke dynamic ftrace guoren
2020-07-21 1:10 ` Palmer Dabbelt
2020-07-09 2:19 ` [PATCH v2 2/6] RISC-V: Implement ptrace regs and stack API guoren
2020-07-09 2:19 ` [PATCH v2 3/6] riscv: Fixup compile error BUILD_BUG_ON failed guoren
2020-07-09 2:19 ` [PATCH v2 4/6] riscv: Add kprobes supported guoren
2020-07-09 2:19 ` [PATCH v2 5/6] riscv: Add uprobes supported guoren
2020-07-09 2:19 ` [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported guoren
2020-07-10 13:50 ` Masami Hiramatsu [this message]
2020-07-11 1:32 ` Guo Ren
2020-07-12 13:37 ` Masami Hiramatsu
2020-07-13 23:47 ` Guo Ren
2020-07-14 11:32 ` Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200710225017.5ce329485e911f99e17cd483@kernel.org \
--to=mhiramat@kernel.org \
--cc=anup@brainfault.org \
--cc=atish.patra@wdc.com \
--cc=bjorn.topel@gmail.com \
--cc=greentime.hu@sifive.com \
--cc=guoren@kernel.org \
--cc=guoren@linux.alibaba.com \
--cc=linux-csky@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=me@packi.ch \
--cc=palmerdabbelt@google.com \
--cc=paul.walmsley@sifive.com \
--cc=penberg@kernel.org \
--cc=zong.li@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).