Re: [PATCH 2/2] tracing: kprobe: make gen test module work in arm and riscv

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Yipeng Zou <zouyipeng@huawei.com>
Cc: <linux-kernel@vger.kernel.org>, <linux-riscv@lists.infradead.org>,
	<rostedt@goodmis.org>, <mingo@redhat.com>,
	<paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
	<aou@eecs.berkeley.edu>, <zanussi@kernel.org>,
	<liaochang1@huawei.com>, <chris.zjh@huawei.com>
Subject: Re: [PATCH 2/2] tracing: kprobe: make gen test module work in arm and riscv
Date: Fri, 23 Sep 2022 23:05:27 +0900	[thread overview]
Message-ID: <20220923230527.6e3ac417a749ed22302d42ab@kernel.org> (raw)
In-Reply-To: <20220919125629.238242-3-zouyipeng@huawei.com>

On Mon, 19 Sep 2022 20:56:29 +0800
Yipeng Zou <zouyipeng@huawei.com> wrote:

> For now, this selftest module can only work in x86 because of the
> kprobe cmd was fixed use of x86 registers.
> This patch adapted to register names under arm and riscv, So that
> this module can be worked on those platform.

This also looks good to me. (good & simple solution :))

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> 
> Fixes: 64836248dda2 ("tracing: Add kprobe event command generation test module")
> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
> ---
>  kernel/trace/kprobe_event_gen_test.c | 47 +++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/kprobe_event_gen_test.c b/kernel/trace/kprobe_event_gen_test.c
> index e023154be0f8..80e04a1e1977 100644
> --- a/kernel/trace/kprobe_event_gen_test.c
> +++ b/kernel/trace/kprobe_event_gen_test.c
> @@ -35,6 +35,45 @@
>  static struct trace_event_file *gen_kprobe_test;
>  static struct trace_event_file *gen_kretprobe_test;
>  
> +#define KPROBE_GEN_TEST_FUNC	"do_sys_open"
> +
> +/* X86 */
> +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_32)
> +#define KPROBE_GEN_TEST_ARG0	"dfd=%ax"
> +#define KPROBE_GEN_TEST_ARG1	"filename=%dx"
> +#define KPROBE_GEN_TEST_ARG2	"flags=%cx"
> +#define KPROBE_GEN_TEST_ARG3	"mode=+4($stack)"
> +
> +/* ARM64 */
> +#elif defined(CONFIG_ARM64)
> +#define KPROBE_GEN_TEST_ARG0	"dfd=%x0"
> +#define KPROBE_GEN_TEST_ARG1	"filename=%x1"
> +#define KPROBE_GEN_TEST_ARG2	"flags=%x2"
> +#define KPROBE_GEN_TEST_ARG3	"mode=%x3"
> +
> +/* ARM */
> +#elif defined(CONFIG_ARM)
> +#define KPROBE_GEN_TEST_ARG0	"dfd=%r0"
> +#define KPROBE_GEN_TEST_ARG1	"filename=%r1"
> +#define KPROBE_GEN_TEST_ARG2	"flags=%r2"
> +#define KPROBE_GEN_TEST_ARG3	"mode=%r3"
> +
> +/* RISCV */
> +#elif defined(CONFIG_RISCV)
> +#define KPROBE_GEN_TEST_ARG0	"dfd=%a0"
> +#define KPROBE_GEN_TEST_ARG1	"filename=%a1"
> +#define KPROBE_GEN_TEST_ARG2	"flags=%a2"
> +#define KPROBE_GEN_TEST_ARG3	"mode=%a3"
> +
> +/* others */
> +#else
> +#define KPROBE_GEN_TEST_ARG0	NULL
> +#define KPROBE_GEN_TEST_ARG1	NULL
> +#define KPROBE_GEN_TEST_ARG2	NULL
> +#define KPROBE_GEN_TEST_ARG3	NULL
> +#endif
> +
> +
>  /*
>   * Test to make sure we can create a kprobe event, then add more
>   * fields.
> @@ -58,14 +97,14 @@ static int __init test_gen_kprobe_cmd(void)
>  	 * fields.
>  	 */
>  	ret = kprobe_event_gen_cmd_start(&cmd, "gen_kprobe_test",
> -					 "do_sys_open",
> -					 "dfd=%ax", "filename=%dx");
> +					 KPROBE_GEN_TEST_FUNC,
> +					 KPROBE_GEN_TEST_ARG0, KPROBE_GEN_TEST_ARG1);
>  	if (ret)
>  		goto free;
>  
>  	/* Use kprobe_event_add_fields to add the rest of the fields */
>  
> -	ret = kprobe_event_add_fields(&cmd, "flags=%cx", "mode=+4($stack)");
> +	ret = kprobe_event_add_fields(&cmd, KPROBE_GEN_TEST_ARG2, KPROBE_GEN_TEST_ARG3);
>  	if (ret)
>  		goto free;
>  
> @@ -128,7 +167,7 @@ static int __init test_gen_kretprobe_cmd(void)
>  	 * Define the kretprobe event.
>  	 */
>  	ret = kretprobe_event_gen_cmd_start(&cmd, "gen_kretprobe_test",
> -					    "do_sys_open",
> +					    KPROBE_GEN_TEST_FUNC,
>  					    "$retval");
>  	if (ret)
>  		goto free;
> -- 
> 2.17.1
> 

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