[RFC PATCH] tracing: BTF testing for kprobe-events

[RFC PATCH] tracing: BTF testing for kprobe-events

[Masami Hiramatsu]

Hi Steve,

Here I share my testing patch of the BTF for kprobe events.
Currently this only allow user to specify '$$args' for
tracing all arguments of the function. This is only
avaialbe if
- the probe point is on the function entry
- the kernel is compiled with BTF (CONFIG_DEBUG_INFO_BTF)
- the kernel is enables BPF (CONFIG_BPF_SYSCALL)

And Special thanks to Sven! Most of BTF handling part of
this patch comes from his patch [1]

[1] https://stackframe.org/0001-ftrace-arg-hack.patch

What I thought while coding this were;
- kernel/bpf/btf.c can be moved under lib/ so that
  the other subsystems can reuse it, independent
  from BPF. (Also, this should depends on CONFIG_DEBUG_INFO_BTF)
- some more utility functions can be exposed.
  e.g. I copied btf_type_int() from btf.c
- If there are more comments for the BTF APIs, it will
  be more useful...
- Overall, the BTF is easy to understand for who
  already understand DWARF. Great work!
- I think I need 'ptr' and 'bool' types for fetcharg types.

Anyway, this is just for testing. I have to add some
more cleanup, features and documentations, etc.

Thank you,

---

Masami Hiramatsu (1):
      tracing/kprobe: Support $$args for function entry


 kernel/trace/trace_kprobe.c |   60 ++++++++++++++++++++++++-
 kernel/trace/trace_probe.c  |  105 +++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h  |    5 ++
 3 files changed, 168 insertions(+), 2 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[RFC PATCH] tracing/kprobe: Support $$args for function entry

[Masami Hiramatsu]

Support $$args fetch arg for function entry. This uses
BTF for finding the function argument. Thus it depends
on CONFIG_BPF_SYSCALL.

/sys/kernel/tracing # echo 'p vfs_read $$args' >> kprobe_events
/sys/kernel/tracing # cat kprobe_events
p:kprobes/p_vfs_read_0 vfs_read file=$arg1:x64 buf=$arg2:x64 count=$arg3:u64 pos=$arg4:x64

Note that $$args must be used without argument name.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_kprobe.c |   60 ++++++++++++++++++++++++-
 kernel/trace/trace_probe.c  |  105 +++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h  |    5 ++
 3 files changed, 168 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3dd4fb719aa3..fe88ee8c8cd8 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -712,6 +712,58 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+#ifdef CONFIG_BPF_SYSCALL
+
+static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
+				       const char *arg, unsigned int flags)
+{
+	struct trace_probe *tp = &tk->tp;
+	static struct btf *btf;
+	const struct btf_type *t;
+	const struct btf_param *args;
+	s32 id, nargs;
+	int ret;
+
+	if (!(flags & TPARG_FL_FENTRY))
+		return -EINVAL;
+	if (!tk->symbol)
+		return -EINVAL;
+
+	if (!btf)
+		btf = btf_parse_vmlinux();
+
+	id = btf_find_by_name_kind(btf, tk->symbol, BTF_KIND_FUNC);
+	if (id <= 0)
+		return -ENOENT;
+
+	/* Get BTF_KIND_FUNC type */
+	t = btf_type_by_id(btf, id);
+	if (!btf_type_is_func(t))
+		return -ENOENT;
+
+	/* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
+	t = btf_type_by_id(btf, t->type);
+	if (!btf_type_is_func_proto(t))
+		return -ENOENT;
+
+	args = (const struct btf_param *)(t + 1);
+	nargs = btf_type_vlen(t);
+	for (i = 0; i < nargs; i++) {
+		ret = traceprobe_parse_btf_arg(tp, i, btf, &args[i]);
+		if (ret < 0)
+			break;
+	}
+
+	return ret;
+}
+#else
+static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
+				       const char *arg, unsigned int flags)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 static struct notifier_block trace_kprobe_module_nb = {
 	.notifier_call = trace_kprobe_module_callback,
 	.priority = 1	/* Invoked after kprobe module callback */
@@ -733,12 +785,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 	 *  $stack	: fetch stack address
 	 *  $stackN	: fetch Nth of stack (N:0-)
 	 *  $comm       : fetch current task comm
+	 *  $$args	: fetch parameters using BTF
 	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
 	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
 	 *  %REG	: fetch register REG
 	 * Dereferencing memory fetch:
 	 *  +|-offs(ARG) : fetch memory at ARG +|- offs address.
-	 * Alias name of args:
+	 * Alias name of args (except for $$args) :
 	 *  NAME=FETCHARG : set NAME as alias of FETCHARG.
 	 * Type of args:
 	 *  FETCHARG:TYPE : use TYPE instead of unsigned long.
@@ -877,7 +930,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 	/* parse arguments */
 	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
 		trace_probe_log_set_index(i + 2);
-		ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
+		if (strcmp(argv[i], "$$args") == 0)
+			ret = trace_kprobe_parse_btf_args(tk, i, argv[i], flags);
+		else
+			ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
 		if (ret)
 			goto error;	/* This can be -ENOMEM */
 	}
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 2fe104109525..bbac261b1688 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -765,6 +765,111 @@ static int traceprobe_conflict_field_name(const char *name,
 	return 0;
 }
 
+#ifdef CONFIG_BPF_SYSCALL
+
+static u32 btf_type_int(const struct btf_type *t)
+{
+	return *(u32 *)(t + 1);
+}
+
+static const char *traceprobe_type_from_btf(struct btf *btf, s32 id)
+{
+	const struct btf_type *t;
+	u32 intdata;
+	s32 tid;
+
+	/* TODO: const char * could be converted as a string */
+	t = btf_type_skip_modifiers(btf, id, &tid);
+
+	switch (BTF_INFO_KIND(t->info)) {
+	case BTF_KIND_ENUM:
+		/* enum is "int", so convert to "s32" */
+		return "s32";
+	case BTF_KIND_PTR:
+		/* pointer will be converted to "x??" */
+		if (IS_ENABLED(CONFIG_64BIT))
+			return "x64";
+		else
+			return "x32";
+	case BTF_KIND_INT:
+		intdata = btf_type_int(t);
+		if (BTF_INT_ENCODING(intdata) & BTF_INT_SIGNED) {
+			switch (BTF_INT_BITS(intdata)) {
+			case 8:
+				return "s8";
+			case 16:
+				return "s16";
+			case 32:
+				return "s32";
+			case 64:
+				return "s64";
+			}
+		} else {	/* unsigned */
+			switch (BTF_INT_BITS(intdata)) {
+			case 8:
+				return "u8";
+			case 16:
+				return "u16";
+			case 32:
+				return "u32";
+			case 64:
+				return "u64";
+			}
+		}
+	}
+
+	/* Default type */
+	if (IS_ENABLED(CONFIG_64BIT))
+		return "x64";
+	else
+		return "x32";
+}
+
+int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
+			     const struct btf_param *arg)
+{
+	struct probe_arg *parg = &tp->args[i];
+	const char *name, *tname;
+	char *body;
+	int ret;
+
+	tp->nr_args++;
+	name = btf_name_by_offset(btf, arg->name_off);
+	parg->name = kstrdup(name, GFP_KERNEL);
+	if (!parg->name)
+		return -ENOMEM;
+
+	if (!is_good_name(parg->name)) {
+		trace_probe_log_err(0, BAD_ARG_NAME);
+		return -EINVAL;
+	}
+	if (traceprobe_conflict_field_name(parg->name, tp->args, i)) {
+		trace_probe_log_err(0, USED_ARG_NAME);
+		return -EINVAL;
+	}
+
+	/*
+	 * Since probe event needs an appropriate command for dyn_event interface,
+	 * convert BTF type to corresponding fetch-type string.
+	 */
+	tname = traceprobe_type_from_btf(btf, arg->type);
+	if (tname)
+		body = kasprintf(GFP_KERNEL, "$arg%d:%s", i + 1, tname);
+	else
+		body = kasprintf(GFP_KERNEL, "$arg%d", i + 1);
+
+	if (!body)
+		return -ENOMEM;
+	/* Parse fetch argument */
+	ret = traceprobe_parse_probe_arg_body(body, &tp->size, parg,
+				TPARG_FL_KERNEL | TPARG_FL_FENTRY, 0);
+
+	kfree(body);
+
+	return ret;
+}
+#endif
+
 int traceprobe_parse_probe_arg(struct trace_probe *tp, int i, const char *arg,
 				unsigned int flags)
 {
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 355c78a930f8..857b946afe29 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -23,6 +23,7 @@
 #include <linux/limits.h>
 #include <linux/uaccess.h>
 #include <linux/bitops.h>
+#include <linux/btf.h>
 #include <asm/bitsperlong.h>
 
 #include "trace.h"
@@ -359,6 +360,10 @@ int trace_probe_create(const char *raw_command, int (*createfn)(int, const char
 
 extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
 				const char *argv, unsigned int flags);
+#ifdef CONFIG_BPF_SYSCALL
+int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
+			     const struct btf_param *arg);
+#endif
 
 extern int traceprobe_update_arg(struct probe_arg *arg);
 extern void traceprobe_free_probe_arg(struct probe_arg *arg);

Re: [RFC PATCH] tracing/kprobe: Support $$args for function entry

[Alan Maguire]

On 23/09/2021 13:39, Masami Hiramatsu wrote:

> Support $$args fetch arg for function entry. This uses
> BTF for finding the function argument. Thus it depends
> on CONFIG_BPF_SYSCALL.
>
> /sys/kernel/tracing # echo 'p vfs_read $$args' >> kprobe_events
> /sys/kernel/tracing # cat kprobe_events
> p:kprobes/p_vfs_read_0 vfs_read file=$arg1:x64 buf=$arg2:x64 count=$arg3:u64 pos=$arg4:x64
>
> Note that $$args must be used without argument name.

This looks great! Can I ask which tree you're building on

top of so I can play around with this a bit?


I also wonder if we could rework btf_show_name() to help

render full type info for the args? in kernel/bpf/btf.c:


/*
  * Populate show->state.name with type name information.
  * Format of type name is
  *
  * [.member_name = ] (type_name)
  */

The (type_name) part is what we'd want from here; no reason

we can't refactor that function to make the type name available

as a cast. It would rework the output to be something like


p:kprobes/p_vfs_read_0 vfs_read struct file *file=$arg1 , char *buf=$arg2 , size_t count=$arg3 , loff_t pos=$arg4

...if that's wanted of course (not sure what the constraints on format are here)? Thanks for pushing this along!


Alan

> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>   kernel/trace/trace_kprobe.c |   60 ++++++++++++++++++++++++-
>   kernel/trace/trace_probe.c  |  105 +++++++++++++++++++++++++++++++++++++++++++
>   kernel/trace/trace_probe.h  |    5 ++
>   3 files changed, 168 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3dd4fb719aa3..fe88ee8c8cd8 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -712,6 +712,58 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
>   	return NOTIFY_DONE;
>   }
>   
> +#ifdef CONFIG_BPF_SYSCALL
> +
> +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> +				       const char *arg, unsigned int flags)
> +{
> +	struct trace_probe *tp = &tk->tp;
> +	static struct btf *btf;
> +	const struct btf_type *t;
> +	const struct btf_param *args;
> +	s32 id, nargs;
> +	int ret;
> +
> +	if (!(flags & TPARG_FL_FENTRY))
> +		return -EINVAL;
> +	if (!tk->symbol)
> +		return -EINVAL;
> +
> +	if (!btf)
> +		btf = btf_parse_vmlinux();
> +
> +	id = btf_find_by_name_kind(btf, tk->symbol, BTF_KIND_FUNC);
> +	if (id <= 0)
> +		return -ENOENT;
> +
> +	/* Get BTF_KIND_FUNC type */
> +	t = btf_type_by_id(btf, id);
> +	if (!btf_type_is_func(t))
> +		return -ENOENT;
> +
> +	/* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> +	t = btf_type_by_id(btf, t->type);
> +	if (!btf_type_is_func_proto(t))
> +		return -ENOENT;
> +
> +	args = (const struct btf_param *)(t + 1);
> +	nargs = btf_type_vlen(t);
> +	for (i = 0; i < nargs; i++) {
> +		ret = traceprobe_parse_btf_arg(tp, i, btf, &args[i]);
> +		if (ret < 0)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +#else
> +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> +				       const char *arg, unsigned int flags)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
> +
>   static struct notifier_block trace_kprobe_module_nb = {
>   	.notifier_call = trace_kprobe_module_callback,
>   	.priority = 1	/* Invoked after kprobe module callback */
> @@ -733,12 +785,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>   	 *  $stack	: fetch stack address
>   	 *  $stackN	: fetch Nth of stack (N:0-)
>   	 *  $comm       : fetch current task comm
> +	 *  $$args	: fetch parameters using BTF
>   	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
>   	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
>   	 *  %REG	: fetch register REG
>   	 * Dereferencing memory fetch:
>   	 *  +|-offs(ARG) : fetch memory at ARG +|- offs address.
> -	 * Alias name of args:
> +	 * Alias name of args (except for $$args) :
>   	 *  NAME=FETCHARG : set NAME as alias of FETCHARG.
>   	 * Type of args:
>   	 *  FETCHARG:TYPE : use TYPE instead of unsigned long.
> @@ -877,7 +930,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>   	/* parse arguments */
>   	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
>   		trace_probe_log_set_index(i + 2);
> -		ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> +		if (strcmp(argv[i], "$$args") == 0)
> +			ret = trace_kprobe_parse_btf_args(tk, i, argv[i], flags);
> +		else
> +			ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
>   		if (ret)
>   			goto error;	/* This can be -ENOMEM */
>   	}
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 2fe104109525..bbac261b1688 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -765,6 +765,111 @@ static int traceprobe_conflict_field_name(const char *name,
>   	return 0;
>   }
>   
> +#ifdef CONFIG_BPF_SYSCALL
> +
> +static u32 btf_type_int(const struct btf_type *t)
> +{
> +	return *(u32 *)(t + 1);
> +}
> +
> +static const char *traceprobe_type_from_btf(struct btf *btf, s32 id)
> +{
> +	const struct btf_type *t;
> +	u32 intdata;
> +	s32 tid;
> +
> +	/* TODO: const char * could be converted as a string */
> +	t = btf_type_skip_modifiers(btf, id, &tid);
> +
> +	switch (BTF_INFO_KIND(t->info)) {
> +	case BTF_KIND_ENUM:
> +		/* enum is "int", so convert to "s32" */
> +		return "s32";
> +	case BTF_KIND_PTR:
> +		/* pointer will be converted to "x??" */
> +		if (IS_ENABLED(CONFIG_64BIT))
> +			return "x64";
> +		else
> +			return "x32";
> +	case BTF_KIND_INT:
> +		intdata = btf_type_int(t);
> +		if (BTF_INT_ENCODING(intdata) & BTF_INT_SIGNED) {
> +			switch (BTF_INT_BITS(intdata)) {
> +			case 8:
> +				return "s8";
> +			case 16:
> +				return "s16";
> +			case 32:
> +				return "s32";
> +			case 64:
> +				return "s64";
> +			}
> +		} else {	/* unsigned */
> +			switch (BTF_INT_BITS(intdata)) {
> +			case 8:
> +				return "u8";
> +			case 16:
> +				return "u16";
> +			case 32:
> +				return "u32";
> +			case 64:
> +				return "u64";
> +			}
> +		}
> +	}
> +
> +	/* Default type */
> +	if (IS_ENABLED(CONFIG_64BIT))
> +		return "x64";
> +	else
> +		return "x32";
> +}
> +
> +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> +			     const struct btf_param *arg)
> +{
> +	struct probe_arg *parg = &tp->args[i];
> +	const char *name, *tname;
> +	char *body;
> +	int ret;
> +
> +	tp->nr_args++;
> +	name = btf_name_by_offset(btf, arg->name_off);
> +	parg->name = kstrdup(name, GFP_KERNEL);
> +	if (!parg->name)
> +		return -ENOMEM;
> +
> +	if (!is_good_name(parg->name)) {
> +		trace_probe_log_err(0, BAD_ARG_NAME);
> +		return -EINVAL;
> +	}
> +	if (traceprobe_conflict_field_name(parg->name, tp->args, i)) {
> +		trace_probe_log_err(0, USED_ARG_NAME);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Since probe event needs an appropriate command for dyn_event interface,
> +	 * convert BTF type to corresponding fetch-type string.
> +	 */
> +	tname = traceprobe_type_from_btf(btf, arg->type);
> +	if (tname)
> +		body = kasprintf(GFP_KERNEL, "$arg%d:%s", i + 1, tname);
> +	else
> +		body = kasprintf(GFP_KERNEL, "$arg%d", i + 1);
> +
> +	if (!body)
> +		return -ENOMEM;
> +	/* Parse fetch argument */
> +	ret = traceprobe_parse_probe_arg_body(body, &tp->size, parg,
> +				TPARG_FL_KERNEL | TPARG_FL_FENTRY, 0);
> +
> +	kfree(body);
> +
> +	return ret;
> +}
> +#endif
> +
>   int traceprobe_parse_probe_arg(struct trace_probe *tp, int i, const char *arg,
>   				unsigned int flags)
>   {
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 355c78a930f8..857b946afe29 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -23,6 +23,7 @@
>   #include <linux/limits.h>
>   #include <linux/uaccess.h>
>   #include <linux/bitops.h>
> +#include <linux/btf.h>
>   #include <asm/bitsperlong.h>
>   
>   #include "trace.h"
> @@ -359,6 +360,10 @@ int trace_probe_create(const char *raw_command, int (*createfn)(int, const char
>   
>   extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
>   				const char *argv, unsigned int flags);
> +#ifdef CONFIG_BPF_SYSCALL
> +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> +			     const struct btf_param *arg);
> +#endif
>   
>   extern int traceprobe_update_arg(struct probe_arg *arg);
>   extern void traceprobe_free_probe_arg(struct probe_arg *arg);
>

Re: [RFC PATCH] tracing/kprobe: Support $$args for function entry

[Masami Hiramatsu]

On Fri, 24 Sep 2021 18:51:04 +0100
Alan Maguire <alan.maguire@oracle.com> wrote:

> On 23/09/2021 13:39, Masami Hiramatsu wrote:
> 
> > Support $$args fetch arg for function entry. This uses
> > BTF for finding the function argument. Thus it depends
> > on CONFIG_BPF_SYSCALL.
> >
> > /sys/kernel/tracing # echo 'p vfs_read $$args' >> kprobe_events
> > /sys/kernel/tracing # cat kprobe_events
> > p:kprobes/p_vfs_read_0 vfs_read file=$arg1:x64 buf=$arg2:x64 count=$arg3:u64 pos=$arg4:x64
> >
> > Note that $$args must be used without argument name.
> 
> This looks great! Can I ask which tree you're building on
> 
> top of so I can play around with this a bit?

This can be applied on top of Steve's tracing tree, ftrace/core branch.

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git

> 
> 
> I also wonder if we could rework btf_show_name() to help
> 
> render full type info for the args? in kernel/bpf/btf.c:
> 
> 
> /*
>   * Populate show->state.name with type name information.
>   * Format of type name is
>   *
>   * [.member_name = ] (type_name)
>   */
> 
> The (type_name) part is what we'd want from here; no reason
> 
> we can't refactor that function to make the type name available
> 
> as a cast. It would rework the output to be something like
> 
> 
> p:kprobes/p_vfs_read_0 vfs_read struct file *file=$arg1 , char *buf=$arg2 , size_t count=$arg3 , loff_t pos=$arg4
> 
> ...if that's wanted of course (not sure what the constraints on format are here)? Thanks for pushing this along!

Sorry, the kprobe_events interface syntax doesn't accept such cast info.
And I want such BTF type query interface to be independent from
this kprobe-events interface.
Can we have a query interface like /sys/kernel/btf/query?

Thank you,

> 
> 
> Alan
> 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >   kernel/trace/trace_kprobe.c |   60 ++++++++++++++++++++++++-
> >   kernel/trace/trace_probe.c  |  105 +++++++++++++++++++++++++++++++++++++++++++
> >   kernel/trace/trace_probe.h  |    5 ++
> >   3 files changed, 168 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 3dd4fb719aa3..fe88ee8c8cd8 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -712,6 +712,58 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
> >   	return NOTIFY_DONE;
> >   }
> >   
> > +#ifdef CONFIG_BPF_SYSCALL
> > +
> > +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> > +				       const char *arg, unsigned int flags)
> > +{
> > +	struct trace_probe *tp = &tk->tp;
> > +	static struct btf *btf;
> > +	const struct btf_type *t;
> > +	const struct btf_param *args;
> > +	s32 id, nargs;
> > +	int ret;
> > +
> > +	if (!(flags & TPARG_FL_FENTRY))
> > +		return -EINVAL;
> > +	if (!tk->symbol)
> > +		return -EINVAL;
> > +
> > +	if (!btf)
> > +		btf = btf_parse_vmlinux();
> > +
> > +	id = btf_find_by_name_kind(btf, tk->symbol, BTF_KIND_FUNC);
> > +	if (id <= 0)
> > +		return -ENOENT;
> > +
> > +	/* Get BTF_KIND_FUNC type */
> > +	t = btf_type_by_id(btf, id);
> > +	if (!btf_type_is_func(t))
> > +		return -ENOENT;
> > +
> > +	/* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> > +	t = btf_type_by_id(btf, t->type);
> > +	if (!btf_type_is_func_proto(t))
> > +		return -ENOENT;
> > +
> > +	args = (const struct btf_param *)(t + 1);
> > +	nargs = btf_type_vlen(t);
> > +	for (i = 0; i < nargs; i++) {
> > +		ret = traceprobe_parse_btf_arg(tp, i, btf, &args[i]);
> > +		if (ret < 0)
> > +			break;
> > +	}
> > +
> > +	return ret;
> > +}
> > +#else
> > +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> > +				       const char *arg, unsigned int flags)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +#endif
> > +
> >   static struct notifier_block trace_kprobe_module_nb = {
> >   	.notifier_call = trace_kprobe_module_callback,
> >   	.priority = 1	/* Invoked after kprobe module callback */
> > @@ -733,12 +785,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> >   	 *  $stack	: fetch stack address
> >   	 *  $stackN	: fetch Nth of stack (N:0-)
> >   	 *  $comm       : fetch current task comm
> > +	 *  $$args	: fetch parameters using BTF
> >   	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
> >   	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
> >   	 *  %REG	: fetch register REG
> >   	 * Dereferencing memory fetch:
> >   	 *  +|-offs(ARG) : fetch memory at ARG +|- offs address.
> > -	 * Alias name of args:
> > +	 * Alias name of args (except for $$args) :
> >   	 *  NAME=FETCHARG : set NAME as alias of FETCHARG.
> >   	 * Type of args:
> >   	 *  FETCHARG:TYPE : use TYPE instead of unsigned long.
> > @@ -877,7 +930,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> >   	/* parse arguments */
> >   	for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
> >   		trace_probe_log_set_index(i + 2);
> > -		ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> > +		if (strcmp(argv[i], "$$args") == 0)
> > +			ret = trace_kprobe_parse_btf_args(tk, i, argv[i], flags);
> > +		else
> > +			ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> >   		if (ret)
> >   			goto error;	/* This can be -ENOMEM */
> >   	}
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index 2fe104109525..bbac261b1688 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -765,6 +765,111 @@ static int traceprobe_conflict_field_name(const char *name,
> >   	return 0;
> >   }
> >   
> > +#ifdef CONFIG_BPF_SYSCALL
> > +
> > +static u32 btf_type_int(const struct btf_type *t)
> > +{
> > +	return *(u32 *)(t + 1);
> > +}
> > +
> > +static const char *traceprobe_type_from_btf(struct btf *btf, s32 id)
> > +{
> > +	const struct btf_type *t;
> > +	u32 intdata;
> > +	s32 tid;
> > +
> > +	/* TODO: const char * could be converted as a string */
> > +	t = btf_type_skip_modifiers(btf, id, &tid);
> > +
> > +	switch (BTF_INFO_KIND(t->info)) {
> > +	case BTF_KIND_ENUM:
> > +		/* enum is "int", so convert to "s32" */
> > +		return "s32";
> > +	case BTF_KIND_PTR:
> > +		/* pointer will be converted to "x??" */
> > +		if (IS_ENABLED(CONFIG_64BIT))
> > +			return "x64";
> > +		else
> > +			return "x32";
> > +	case BTF_KIND_INT:
> > +		intdata = btf_type_int(t);
> > +		if (BTF_INT_ENCODING(intdata) & BTF_INT_SIGNED) {
> > +			switch (BTF_INT_BITS(intdata)) {
> > +			case 8:
> > +				return "s8";
> > +			case 16:
> > +				return "s16";
> > +			case 32:
> > +				return "s32";
> > +			case 64:
> > +				return "s64";
> > +			}
> > +		} else {	/* unsigned */
> > +			switch (BTF_INT_BITS(intdata)) {
> > +			case 8:
> > +				return "u8";
> > +			case 16:
> > +				return "u16";
> > +			case 32:
> > +				return "u32";
> > +			case 64:
> > +				return "u64";
> > +			}
> > +		}
> > +	}
> > +
> > +	/* Default type */
> > +	if (IS_ENABLED(CONFIG_64BIT))
> > +		return "x64";
> > +	else
> > +		return "x32";
> > +}
> > +
> > +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> > +			     const struct btf_param *arg)
> > +{
> > +	struct probe_arg *parg = &tp->args[i];
> > +	const char *name, *tname;
> > +	char *body;
> > +	int ret;
> > +
> > +	tp->nr_args++;
> > +	name = btf_name_by_offset(btf, arg->name_off);
> > +	parg->name = kstrdup(name, GFP_KERNEL);
> > +	if (!parg->name)
> > +		return -ENOMEM;
> > +
> > +	if (!is_good_name(parg->name)) {
> > +		trace_probe_log_err(0, BAD_ARG_NAME);
> > +		return -EINVAL;
> > +	}
> > +	if (traceprobe_conflict_field_name(parg->name, tp->args, i)) {
> > +		trace_probe_log_err(0, USED_ARG_NAME);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * Since probe event needs an appropriate command for dyn_event interface,
> > +	 * convert BTF type to corresponding fetch-type string.
> > +	 */
> > +	tname = traceprobe_type_from_btf(btf, arg->type);
> > +	if (tname)
> > +		body = kasprintf(GFP_KERNEL, "$arg%d:%s", i + 1, tname);
> > +	else
> > +		body = kasprintf(GFP_KERNEL, "$arg%d", i + 1);
> > +
> > +	if (!body)
> > +		return -ENOMEM;
> > +	/* Parse fetch argument */
> > +	ret = traceprobe_parse_probe_arg_body(body, &tp->size, parg,
> > +				TPARG_FL_KERNEL | TPARG_FL_FENTRY, 0);
> > +
> > +	kfree(body);
> > +
> > +	return ret;
> > +}
> > +#endif
> > +
> >   int traceprobe_parse_probe_arg(struct trace_probe *tp, int i, const char *arg,
> >   				unsigned int flags)
> >   {
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index 355c78a930f8..857b946afe29 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -23,6 +23,7 @@
> >   #include <linux/limits.h>
> >   #include <linux/uaccess.h>
> >   #include <linux/bitops.h>
> > +#include <linux/btf.h>
> >   #include <asm/bitsperlong.h>
> >   
> >   #include "trace.h"
> > @@ -359,6 +360,10 @@ int trace_probe_create(const char *raw_command, int (*createfn)(int, const char
> >   
> >   extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
> >   				const char *argv, unsigned int flags);
> > +#ifdef CONFIG_BPF_SYSCALL
> > +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> > +			     const struct btf_param *arg);
> > +#endif
> >   
> >   extern int traceprobe_update_arg(struct probe_arg *arg);
> >   extern void traceprobe_free_probe_arg(struct probe_arg *arg);
> >


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [RFC PATCH] tracing: BTF testing for kprobe-events

[Steven Rostedt]

On Thu, 23 Sep 2021 21:39:43 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi Steve,
> 

Hi Masami,

Sorry for the late reply, but Plumbers followed by OSS put me way behind,
and I just got to this email :-/

> Here I share my testing patch of the BTF for kprobe events.
> Currently this only allow user to specify '$$args' for
> tracing all arguments of the function. This is only
> avaialbe if
> - the probe point is on the function entry
> - the kernel is compiled with BTF (CONFIG_DEBUG_INFO_BTF)
> - the kernel is enables BPF (CONFIG_BPF_SYSCALL)
> 
> And Special thanks to Sven! Most of BTF handling part of
> this patch comes from his patch [1]
> 
> [1] https://stackframe.org/0001-ftrace-arg-hack.patch

Which is newer than this patch because he sent a v2, and that's a couple
patches down in my queue. I'll be looking at that one shortly as well.

> 
> What I thought while coding this were;
> - kernel/bpf/btf.c can be moved under lib/ so that
>   the other subsystems can reuse it, independent
>   from BPF. (Also, this should depends on CONFIG_DEBUG_INFO_BTF)

Makes sense.

> - some more utility functions can be exposed.
>   e.g. I copied btf_type_int() from btf.c

Agreed.

> - If there are more comments for the BTF APIs, it will
>   be more useful...
> - Overall, the BTF is easy to understand for who
>   already understand DWARF. Great work!

Great to hear.

> - I think I need 'ptr' and 'bool' types for fetcharg types.
> 
> Anyway, this is just for testing. I have to add some
> more cleanup, features and documentations, etc.

This is awesome, and something to look at for a generic ftrace args point
of view too.

One issue is how do we handle multiple register values? Like a u64 type on
32 bit?  As $arg1 is just a register that is in $arg1, for a u64 parameter
on 32 bit, that is usually handled with two registers.

Have thoughts on that?

I'll play with your patch today.

-- Steve

Re: [RFC PATCH] tracing: BTF testing for kprobe-events

[Masami Hiramatsu]

On Mon, 11 Oct 2021 18:23:34 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 23 Sep 2021 21:39:43 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Hi Steve,
> > 
> 
> Hi Masami,
> 
> Sorry for the late reply, but Plumbers followed by OSS put me way behind,
> and I just got to this email :-/
> 
> > Here I share my testing patch of the BTF for kprobe events.
> > Currently this only allow user to specify '$$args' for
> > tracing all arguments of the function. This is only
> > avaialbe if
> > - the probe point is on the function entry
> > - the kernel is compiled with BTF (CONFIG_DEBUG_INFO_BTF)
> > - the kernel is enables BPF (CONFIG_BPF_SYSCALL)
> > 
> > And Special thanks to Sven! Most of BTF handling part of
> > this patch comes from his patch [1]
> > 
> > [1] https://stackframe.org/0001-ftrace-arg-hack.patch
> 
> Which is newer than this patch because he sent a v2, and that's a couple
> patches down in my queue. I'll be looking at that one shortly as well.

Did he send his BTF hack patch to you ?
I didn't notice that.


> > What I thought while coding this were;
> > - kernel/bpf/btf.c can be moved under lib/ so that
> >   the other subsystems can reuse it, independent
> >   from BPF. (Also, this should depends on CONFIG_DEBUG_INFO_BTF)
> 
> Makes sense.
> 
> > - some more utility functions can be exposed.
> >   e.g. I copied btf_type_int() from btf.c
> 
> Agreed.
> 
> > - If there are more comments for the BTF APIs, it will
> >   be more useful...
> > - Overall, the BTF is easy to understand for who
> >   already understand DWARF. Great work!
> 
> Great to hear.
> 
> > - I think I need 'ptr' and 'bool' types for fetcharg types.
> > 
> > Anyway, this is just for testing. I have to add some
> > more cleanup, features and documentations, etc.
> 
> This is awesome, and something to look at for a generic ftrace args point
> of view too.
> 
> One issue is how do we handle multiple register values? Like a u64 type on
> 32 bit?  As $arg1 is just a register that is in $arg1, for a u64 parameter
> on 32 bit, that is usually handled with two registers.
> 
> Have thoughts on that?

Oh, that's a good point! The probe event supports such case, since I expected
the user will use 2 arguments to record it. But indeed, using BTF means we need
such extension.
OK, let me consider how to extend fetchargs to support it.

Thank you!

> 
> I'll play with your patch today.
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>