* [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64
@ 2024-08-31 4:19 Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 1/4] libbpf: Access first syscall argument with CO-RE direct read on s390 Pu Lehui
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Pu Lehui @ 2024-08-31 4:19 UTC (permalink / raw)
To: bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On RV64, as Ilya mentioned before [0], the first syscall parameter should be
accessed through orig_a0 (see arch/riscv64/include/asm/syscall.h),
otherwise it will cause selftests like bpf_syscall_macro, vmlinux,
test_lsm, etc. to fail on RV64.
Link: https://lore.kernel.org/bpf/20220209021745.2215452-1-iii@linux.ibm.com [0]
v3:
- Fix test case error.
v2: https://lore.kernel.org/all/20240831023646.1558629-1-pulehui@huaweicloud.com/
- Access first syscall argument with CO-RE direct read. (Andrii)
v1: https://lore.kernel.org/all/20240829133453.882259-1-pulehui@huaweicloud.com/
Pu Lehui (4):
libbpf: Access first syscall argument with CO-RE direct read on s390
libbpf: Access first syscall argument with CO-RE direct read on arm64
selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and
arm64
libbpf: Fix accessing first syscall argument on RV64
tools/lib/bpf/bpf_tracing.h | 17 ++++++++++++-----
.../bpf/prog_tests/test_bpf_syscall_macro.c | 4 ----
.../selftests/bpf/progs/bpf_syscall_macro.c | 2 --
3 files changed, 12 insertions(+), 11 deletions(-)
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH bpf-next v3 1/4] libbpf: Access first syscall argument with CO-RE direct read on s390
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
@ 2024-08-31 4:19 ` Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64 Pu Lehui
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Pu Lehui @ 2024-08-31 4:19 UTC (permalink / raw)
To: bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
into account the read pt_regs comes directly from the context, let's use
CO-RE direct read to access the first system call argument.
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
tools/lib/bpf/bpf_tracing.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 9314fa95f04e..e7d9382efeb3 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -163,7 +163,7 @@
struct pt_regs___s390 {
unsigned long orig_gpr2;
-};
+} __attribute__((preserve_access_index));
/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
#define __PT_REGS_CAST(x) ((const user_pt_regs *)(x))
@@ -179,7 +179,7 @@ struct pt_regs___s390 {
#define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
#define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
#define __PT_PARM6_SYSCALL_REG gprs[7]
-#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
+#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___s390 *)(x))->orig_gpr2)
#define PT_REGS_PARM1_CORE_SYSCALL(x) \
BPF_CORE_READ((const struct pt_regs___s390 *)(x), __PT_PARM1_SYSCALL_REG)
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 1/4] libbpf: Access first syscall argument with CO-RE direct read on s390 Pu Lehui
@ 2024-08-31 4:19 ` Pu Lehui
2024-08-31 7:26 ` Xu Kuohai
2024-09-04 20:21 ` Andrii Nakryiko
2024-08-31 4:19 ` [PATCH bpf-next v3 3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64 Pu Lehui
` (3 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Pu Lehui @ 2024-08-31 4:19 UTC (permalink / raw)
To: bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
into account the read pt_regs comes directly from the context, let's use
CO-RE direct read to access the first system call argument.
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
tools/lib/bpf/bpf_tracing.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index e7d9382efeb3..051c408e6aed 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -222,7 +222,7 @@ struct pt_regs___s390 {
struct pt_regs___arm64 {
unsigned long orig_x0;
-};
+} __attribute__((preserve_access_index));
/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
#define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
@@ -241,7 +241,7 @@ struct pt_regs___arm64 {
#define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
#define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
#define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
-#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
+#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
#define PT_REGS_PARM1_CORE_SYSCALL(x) \
BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH bpf-next v3 3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 1/4] libbpf: Access first syscall argument with CO-RE direct read on s390 Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64 Pu Lehui
@ 2024-08-31 4:19 ` Pu Lehui
2024-09-04 20:21 ` Andrii Nakryiko
2024-08-31 4:19 ` [PATCH bpf-next v3 4/4] libbpf: Fix accessing first syscall argument on RV64 Pu Lehui
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Pu Lehui @ 2024-08-31 4:19 UTC (permalink / raw)
To: bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
Considering that CO-RE direct read access to the first system call
argument is already available on s390 and arm64, let's enable
test_bpf_syscall_macro:syscall_arg1 on these architectures.
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
.../testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c | 4 ----
tools/testing/selftests/bpf/progs/bpf_syscall_macro.c | 2 --
2 files changed, 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c b/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c
index 2900c5e9a016..1750c29b94f8 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c
@@ -38,11 +38,7 @@ void test_bpf_syscall_macro(void)
/* check whether args of syscall are copied correctly */
prctl(exp_arg1, exp_arg2, exp_arg3, exp_arg4, exp_arg5);
-#if defined(__aarch64__) || defined(__s390__)
- ASSERT_NEQ(skel->bss->arg1, exp_arg1, "syscall_arg1");
-#else
ASSERT_EQ(skel->bss->arg1, exp_arg1, "syscall_arg1");
-#endif
ASSERT_EQ(skel->bss->arg2, exp_arg2, "syscall_arg2");
ASSERT_EQ(skel->bss->arg3, exp_arg3, "syscall_arg3");
/* it cannot copy arg4 when uses PT_REGS_PARM4 on x86_64 */
diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
index 1a476d8ed354..9e7d9674ce2a 100644
--- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
+++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
@@ -43,9 +43,7 @@ int BPF_KPROBE(handle_sys_prctl)
/* test for PT_REGS_PARM */
-#if !defined(bpf_target_arm64) && !defined(bpf_target_s390)
bpf_probe_read_kernel(&tmp, sizeof(tmp), &PT_REGS_PARM1_SYSCALL(real_regs));
-#endif
arg1 = tmp;
bpf_probe_read_kernel(&arg2, sizeof(arg2), &PT_REGS_PARM2_SYSCALL(real_regs));
bpf_probe_read_kernel(&arg3, sizeof(arg3), &PT_REGS_PARM3_SYSCALL(real_regs));
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH bpf-next v3 4/4] libbpf: Fix accessing first syscall argument on RV64
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
` (2 preceding siblings ...)
2024-08-31 4:19 ` [PATCH bpf-next v3 3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64 Pu Lehui
@ 2024-08-31 4:19 ` Pu Lehui
2024-09-04 20:30 ` [PATCH bpf-next v3 0/4] " patchwork-bot+netdevbpf
2024-10-01 11:35 ` patchwork-bot+linux-riscv
5 siblings, 0 replies; 16+ messages in thread
From: Pu Lehui @ 2024-08-31 4:19 UTC (permalink / raw)
To: bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
On RV64, as Ilya mentioned before [0], the first syscall parameter should be
accessed through orig_a0 (see arch/riscv64/include/asm/syscall.h),
otherwise it will cause selftests like bpf_syscall_macro, vmlinux,
test_lsm, etc. to fail on RV64. Let's fix it by using the struct pt_regs
style CO-RE direct access.
Link: https://lore.kernel.org/bpf/20220209021745.2215452-1-iii@linux.ibm.com [0]
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
tools/lib/bpf/bpf_tracing.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 051c408e6aed..6a4c49d1ca72 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -351,6 +351,10 @@ struct pt_regs___arm64 {
* https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#risc-v-calling-conventions
*/
+struct pt_regs___riscv {
+ unsigned long orig_a0;
+} __attribute__((preserve_access_index));
+
/* riscv provides struct user_regs_struct instead of struct pt_regs to userspace */
#define __PT_REGS_CAST(x) ((const struct user_regs_struct *)(x))
#define __PT_PARM1_REG a0
@@ -362,12 +366,15 @@ struct pt_regs___arm64 {
#define __PT_PARM7_REG a6
#define __PT_PARM8_REG a7
-#define __PT_PARM1_SYSCALL_REG __PT_PARM1_REG
+#define __PT_PARM1_SYSCALL_REG orig_a0
#define __PT_PARM2_SYSCALL_REG __PT_PARM2_REG
#define __PT_PARM3_SYSCALL_REG __PT_PARM3_REG
#define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
#define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
#define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
+#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___riscv *)(x))->orig_a0)
+#define PT_REGS_PARM1_CORE_SYSCALL(x) \
+ BPF_CORE_READ((const struct pt_regs___riscv *)(x), __PT_PARM1_SYSCALL_REG)
#define __PT_RET_REG ra
#define __PT_FP_REG s0
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-08-31 4:19 ` [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64 Pu Lehui
@ 2024-08-31 7:26 ` Xu Kuohai
2024-08-31 7:57 ` Xu Kuohai
2024-09-04 20:21 ` Andrii Nakryiko
1 sibling, 1 reply; 16+ messages in thread
From: Xu Kuohai @ 2024-08-31 7:26 UTC (permalink / raw)
To: Pu Lehui, bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On 8/31/2024 12:19 PM, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
>
> Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
> SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
> into account the read pt_regs comes directly from the context, let's use
> CO-RE direct read to access the first system call argument.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
> tools/lib/bpf/bpf_tracing.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index e7d9382efeb3..051c408e6aed 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -222,7 +222,7 @@ struct pt_regs___s390 {
>
> struct pt_regs___arm64 {
> unsigned long orig_x0;
> -};
> +} __attribute__((preserve_access_index));
>
> /* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
> #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
> @@ -241,7 +241,7 @@ struct pt_regs___arm64 {
> #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
> #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
> #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
> -#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
> +#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
> #define PT_REGS_PARM1_CORE_SYSCALL(x) \
> BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
>
Cool!
Acked-by: Xu Kuohai <xukuohai@huawei.com>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-08-31 7:26 ` Xu Kuohai
@ 2024-08-31 7:57 ` Xu Kuohai
2024-09-04 20:06 ` Andrii Nakryiko
0 siblings, 1 reply; 16+ messages in thread
From: Xu Kuohai @ 2024-08-31 7:57 UTC (permalink / raw)
To: Pu Lehui, bpf, linux-riscv, netdev, Andrii Nakryiko
Cc: Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On 8/31/2024 3:26 PM, Xu Kuohai wrote:
> On 8/31/2024 12:19 PM, Pu Lehui wrote:
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
>> SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
>> into account the read pt_regs comes directly from the context, let's use
>> CO-RE direct read to access the first system call argument.
>>
>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> ---
>> tools/lib/bpf/bpf_tracing.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
>> index e7d9382efeb3..051c408e6aed 100644
>> --- a/tools/lib/bpf/bpf_tracing.h
>> +++ b/tools/lib/bpf/bpf_tracing.h
>> @@ -222,7 +222,7 @@ struct pt_regs___s390 {
>> struct pt_regs___arm64 {
>> unsigned long orig_x0;
>> -};
>> +} __attribute__((preserve_access_index));
>> /* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
>> #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
>> @@ -241,7 +241,7 @@ struct pt_regs___arm64 {
>> #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
>> #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
>> #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
>> -#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
>> +#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
>> #define PT_REGS_PARM1_CORE_SYSCALL(x) \
>> BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
>
> Cool!
>
> Acked-by: Xu Kuohai <xukuohai@huawei.com>
>
>
Wait, it breaks the following test:
--- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
+++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
@@ -43,7 +43,7 @@ int BPF_KPROBE(handle_sys_prctl)
/* test for PT_REGS_PARM */
- bpf_probe_read_kernel(&tmp, sizeof(tmp), &PT_REGS_PARM1_SYSCALL(real_regs));
+ tmp = PT_REGS_PARM1_SYSCALL(real_regs);
arg1 = tmp;
bpf_probe_read_kernel(&arg2, sizeof(arg2), &PT_REGS_PARM2_SYSCALL(real_regs));
bpf_probe_read_kernel(&arg3, sizeof(arg3), &PT_REGS_PARM3_SYSCALL(real_regs));
Failed with verifier rejection:
0: R1=ctx() R10=fp0
; int BPF_KPROBE(handle_sys_prctl) @ bpf_syscall_macro.c:33
0: (bf) r6 = r1 ; R1=ctx() R6_w=ctx()
; pid_t pid = bpf_get_current_pid_tgid() >> 32; @ bpf_syscall_macro.c:36
1: (85) call bpf_get_current_pid_tgid#14 ; R0_w=scalar()
; if (pid != filter_pid) @ bpf_syscall_macro.c:39
2: (18) r1 = 0xffff800082e0e000 ; R1_w=map_value(map=bpf_sysc.rodata,ks=4,vs=4)
4: (61) r1 = *(u32 *)(r1 +0) ; R1_w=607
; pid_t pid = bpf_get_current_pid_tgid() >> 32; @ bpf_syscall_macro.c:36
5: (77) r0 >>= 32 ; R0_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
; if (pid != filter_pid) @ bpf_syscall_macro.c:39
6: (5e) if w1 != w0 goto pc+98 ; R0_w=607 R1_w=607
; real_regs = PT_REGS_SYSCALL_REGS(ctx); @ bpf_syscall_macro.c:42
7: (79) r8 = *(u64 *)(r6 +0) ; R6_w=ctx() R8_w=scalar()
; tmp = PT_REGS_PARM1_SYSCALL(real_regs); @ bpf_syscall_macro.c:46
8: (79) r1 = *(u64 *)(r8 +272)
R8 invalid mem access 'scalar'
processed 8 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-08-31 7:57 ` Xu Kuohai
@ 2024-09-04 20:06 ` Andrii Nakryiko
2024-09-05 2:39 ` Xu Kuohai
0 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2024-09-04 20:06 UTC (permalink / raw)
To: Xu Kuohai
Cc: Pu Lehui, bpf, linux-riscv, netdev, Andrii Nakryiko,
Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On Sat, Aug 31, 2024 at 12:57 AM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>
> On 8/31/2024 3:26 PM, Xu Kuohai wrote:
> > On 8/31/2024 12:19 PM, Pu Lehui wrote:
> >> From: Pu Lehui <pulehui@huawei.com>
> >>
> >> Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
> >> SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
> >> into account the read pt_regs comes directly from the context, let's use
> >> CO-RE direct read to access the first system call argument.
> >>
> >> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> >> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> >> ---
> >> tools/lib/bpf/bpf_tracing.h | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> >> index e7d9382efeb3..051c408e6aed 100644
> >> --- a/tools/lib/bpf/bpf_tracing.h
> >> +++ b/tools/lib/bpf/bpf_tracing.h
> >> @@ -222,7 +222,7 @@ struct pt_regs___s390 {
> >> struct pt_regs___arm64 {
> >> unsigned long orig_x0;
> >> -};
> >> +} __attribute__((preserve_access_index));
> >> /* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
> >> #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
> >> @@ -241,7 +241,7 @@ struct pt_regs___arm64 {
> >> #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
> >> #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
> >> #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
> >> -#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
> >> +#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
> >> #define PT_REGS_PARM1_CORE_SYSCALL(x) \
> >> BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
> >
> > Cool!
> >
> > Acked-by: Xu Kuohai <xukuohai@huawei.com>
> >
> >
>
> Wait, it breaks the following test:
>
You mean, *if you change the existing test like below*, it will break,
right? And that's expected, because arm64 has
ARCH_HAS_SYSCALL_WRAPPER, which means syscall pt_regs are actually not
the kprobe's ctx, so you can't directly access it. Which is why we
have PT_REGS_PARM1_CORE_SYSCALL() variants.
See how BPF_KSYSCALL macro is implemented, there are two cases:
___bpf_syswap_args(), which uses BPF_CORE_READ()-based macros to fetch
arguments, and ___bpf_syscall_args() which uses direct ctx reads.
> --- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> @@ -43,7 +43,7 @@ int BPF_KPROBE(handle_sys_prctl)
>
> /* test for PT_REGS_PARM */
>
> - bpf_probe_read_kernel(&tmp, sizeof(tmp), &PT_REGS_PARM1_SYSCALL(real_regs));
> + tmp = PT_REGS_PARM1_SYSCALL(real_regs);
> arg1 = tmp;
> bpf_probe_read_kernel(&arg2, sizeof(arg2), &PT_REGS_PARM2_SYSCALL(real_regs));
> bpf_probe_read_kernel(&arg3, sizeof(arg3), &PT_REGS_PARM3_SYSCALL(real_regs));
>
> Failed with verifier rejection:
>
> 0: R1=ctx() R10=fp0
> ; int BPF_KPROBE(handle_sys_prctl) @ bpf_syscall_macro.c:33
> 0: (bf) r6 = r1 ; R1=ctx() R6_w=ctx()
> ; pid_t pid = bpf_get_current_pid_tgid() >> 32; @ bpf_syscall_macro.c:36
> 1: (85) call bpf_get_current_pid_tgid#14 ; R0_w=scalar()
> ; if (pid != filter_pid) @ bpf_syscall_macro.c:39
> 2: (18) r1 = 0xffff800082e0e000 ; R1_w=map_value(map=bpf_sysc.rodata,ks=4,vs=4)
> 4: (61) r1 = *(u32 *)(r1 +0) ; R1_w=607
> ; pid_t pid = bpf_get_current_pid_tgid() >> 32; @ bpf_syscall_macro.c:36
> 5: (77) r0 >>= 32 ; R0_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
> ; if (pid != filter_pid) @ bpf_syscall_macro.c:39
> 6: (5e) if w1 != w0 goto pc+98 ; R0_w=607 R1_w=607
> ; real_regs = PT_REGS_SYSCALL_REGS(ctx); @ bpf_syscall_macro.c:42
> 7: (79) r8 = *(u64 *)(r6 +0) ; R6_w=ctx() R8_w=scalar()
> ; tmp = PT_REGS_PARM1_SYSCALL(real_regs); @ bpf_syscall_macro.c:46
> 8: (79) r1 = *(u64 *)(r8 +272)
> R8 invalid mem access 'scalar'
> processed 8 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-08-31 4:19 ` [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64 Pu Lehui
2024-08-31 7:26 ` Xu Kuohai
@ 2024-09-04 20:21 ` Andrii Nakryiko
2024-09-05 6:42 ` Pu Lehui
1 sibling, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2024-09-04 20:21 UTC (permalink / raw)
To: Pu Lehui
Cc: bpf, linux-riscv, netdev, Andrii Nakryiko, Björn Töpel,
Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Puranjay Mohan,
Palmer Dabbelt, Pu Lehui
On Fri, Aug 30, 2024 at 9:17 PM Pu Lehui <pulehui@huaweicloud.com> wrote:
>
> From: Pu Lehui <pulehui@huawei.com>
>
> Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
> SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
> into account the read pt_regs comes directly from the context, let's use
> CO-RE direct read to access the first system call argument.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
> tools/lib/bpf/bpf_tracing.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index e7d9382efeb3..051c408e6aed 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -222,7 +222,7 @@ struct pt_regs___s390 {
>
> struct pt_regs___arm64 {
> unsigned long orig_x0;
> -};
> +} __attribute__((preserve_access_index));
>
> /* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
> #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
> @@ -241,7 +241,7 @@ struct pt_regs___arm64 {
> #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
> #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
> #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
> -#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
> +#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
It would probably be best (for consistency) to stick to using
__PTR_PARM1_SYSCALL_REG instead of hard-coding orig_x0 here, no? I'll
fix it up while applying. Same for patch #1 and #4.
It would be great if you can double-check that final patches in
bpf-next/master compile and work well for arm64, s390x, and RV64 (as I
can't really test that much locally).
> #define PT_REGS_PARM1_CORE_SYSCALL(x) \
> BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
>
> --
> 2.34.1
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64
2024-08-31 4:19 ` [PATCH bpf-next v3 3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64 Pu Lehui
@ 2024-09-04 20:21 ` Andrii Nakryiko
0 siblings, 0 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2024-09-04 20:21 UTC (permalink / raw)
To: Pu Lehui
Cc: bpf, linux-riscv, netdev, Andrii Nakryiko, Björn Töpel,
Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Puranjay Mohan,
Palmer Dabbelt, Pu Lehui
On Fri, Aug 30, 2024 at 9:17 PM Pu Lehui <pulehui@huaweicloud.com> wrote:
>
> From: Pu Lehui <pulehui@huawei.com>
>
> Considering that CO-RE direct read access to the first system call
> argument is already available on s390 and arm64, let's enable
> test_bpf_syscall_macro:syscall_arg1 on these architectures.
>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
> .../testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c | 4 ----
> tools/testing/selftests/bpf/progs/bpf_syscall_macro.c | 2 --
> 2 files changed, 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c b/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c
> index 2900c5e9a016..1750c29b94f8 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c
> @@ -38,11 +38,7 @@ void test_bpf_syscall_macro(void)
> /* check whether args of syscall are copied correctly */
> prctl(exp_arg1, exp_arg2, exp_arg3, exp_arg4, exp_arg5);
>
> -#if defined(__aarch64__) || defined(__s390__)
> - ASSERT_NEQ(skel->bss->arg1, exp_arg1, "syscall_arg1");
> -#else
> ASSERT_EQ(skel->bss->arg1, exp_arg1, "syscall_arg1");
> -#endif
> ASSERT_EQ(skel->bss->arg2, exp_arg2, "syscall_arg2");
> ASSERT_EQ(skel->bss->arg3, exp_arg3, "syscall_arg3");
> /* it cannot copy arg4 when uses PT_REGS_PARM4 on x86_64 */
> diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> index 1a476d8ed354..9e7d9674ce2a 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
> @@ -43,9 +43,7 @@ int BPF_KPROBE(handle_sys_prctl)
>
> /* test for PT_REGS_PARM */
>
> -#if !defined(bpf_target_arm64) && !defined(bpf_target_s390)
> bpf_probe_read_kernel(&tmp, sizeof(tmp), &PT_REGS_PARM1_SYSCALL(real_regs));
> -#endif
> arg1 = tmp;
There is no point in having tmp variable now, I cleaned that up as well
> bpf_probe_read_kernel(&arg2, sizeof(arg2), &PT_REGS_PARM2_SYSCALL(real_regs));
> bpf_probe_read_kernel(&arg3, sizeof(arg3), &PT_REGS_PARM3_SYSCALL(real_regs));
> --
> 2.34.1
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
` (3 preceding siblings ...)
2024-08-31 4:19 ` [PATCH bpf-next v3 4/4] libbpf: Fix accessing first syscall argument on RV64 Pu Lehui
@ 2024-09-04 20:30 ` patchwork-bot+netdevbpf
[not found] ` <CAEf4BzayV1ihbfSg4fv0AqSazVycXqCJp4dTq1pwRt5hmx7X4g@mail.gmail.com>
2024-10-01 11:35 ` patchwork-bot+linux-riscv
5 siblings, 1 reply; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-09-04 20:30 UTC (permalink / raw)
To: Pu Lehui
Cc: bpf, linux-riscv, netdev, andrii, bjorn, iii, ast, daniel,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, puranjay, palmer, pulehui
Hello:
This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Sat, 31 Aug 2024 04:19:30 +0000 you wrote:
> On RV64, as Ilya mentioned before [0], the first syscall parameter should be
> accessed through orig_a0 (see arch/riscv64/include/asm/syscall.h),
> otherwise it will cause selftests like bpf_syscall_macro, vmlinux,
> test_lsm, etc. to fail on RV64.
>
> Link: https://lore.kernel.org/bpf/20220209021745.2215452-1-iii@linux.ibm.com [0]
>
> [...]
Here is the summary with links:
- [bpf-next,v3,1/4] libbpf: Access first syscall argument with CO-RE direct read on s390
https://git.kernel.org/bpf/bpf-next/c/65ee11d9c822
- [bpf-next,v3,2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
https://git.kernel.org/bpf/bpf-next/c/ebd8ad474888
- [bpf-next,v3,3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64
https://git.kernel.org/bpf/bpf-next/c/3a913c4d62e1
- [bpf-next,v3,4/4] libbpf: Fix accessing first syscall argument on RV64
https://git.kernel.org/bpf/bpf-next/c/13143c5816bc
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64
[not found] ` <CAEf4BzayV1ihbfSg4fv0AqSazVycXqCJp4dTq1pwRt5hmx7X4g@mail.gmail.com>
@ 2024-09-05 0:03 ` Andrii Nakryiko
2024-09-05 2:26 ` Pu Lehui
0 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2024-09-05 0:03 UTC (permalink / raw)
To: patchwork-bot+netdevbpf
Cc: Pu Lehui, bpf, linux-riscv, Networking, Andrii Nakryiko,
Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, john fastabend, KP Singh, Hao Luo, Jiri Olsa,
Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On Wed, Sep 4, 2024 at 3:44 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
>
>
> On Wed, Sep 4, 2024 at 1:30 PM <patchwork-bot+netdevbpf@kernel.org> wrote:
> >
> > Hello:
> >
> > This series was applied to bpf/bpf-next.git (master)
> > by Andrii Nakryiko <andrii@kernel.org>:
> >
> > On Sat, 31 Aug 2024 04:19:30 +0000 you wrote:
> > > On RV64, as Ilya mentioned before [0], the first syscall parameter should be
> > > accessed through orig_a0 (see arch/riscv64/include/asm/syscall.h),
> > > otherwise it will cause selftests like bpf_syscall_macro, vmlinux,
> > > test_lsm, etc. to fail on RV64.
> > >
> > > Link: https://lore.kernel.org/bpf/20220209021745.2215452-1-iii@linux.ibm.com [0]
> > >
> > > [...]
> >
> > Here is the summary with links:
> > - [bpf-next,v3,1/4] libbpf: Access first syscall argument with CO-RE direct read on s390
> > https://git.kernel.org/bpf/bpf-next/c/65ee11d9c822
> > - [bpf-next,v3,2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
> > https://git.kernel.org/bpf/bpf-next/c/ebd8ad474888
> > - [bpf-next,v3,3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64
> > https://git.kernel.org/bpf/bpf-next/c/3a913c4d62e1
> > - [bpf-next,v3,4/4] libbpf: Fix accessing first syscall argument on RV64
> > https://git.kernel.org/bpf/bpf-next/c/13143c5816bc
> >
>
> Ok, I had to "unapply" these patches, as s390x selftest (bpf_syscall_macro) started failing (arm64 is fine, so I think it's probably due to patch #3 that changes selftests itself).
>
> Pu, can you please take a look at that (e.g., see [0])? It's a bit strange, as originally no error was reported, so not sure what changed. Please also see the things I was fixing up while applying, so I don't have to do it again :)
>
> [0] https://github.com/kernel-patches/bpf/actions/runs/10709024755/job/29693056550#step:5:8746
Oh, I figured it out! That tmp is there for a reason! We
bpf_probe_read_kernel() 8 bytes, but syscall's 1st argument itself is
4 byte long, so we need to cast u64 to u32. And s390x being the big
endian architecture detected this problem, while for arm64 we were
lucky.
So never mind, I'll apply your patches with fix ups in the
bpf_tracing.h header, but I won't touch patch #3.
>
> > You are awesome, thank you!
> > --
> > Deet-doot-dot, I am a bot.
> > https://korg.docs.kernel.org/patchwork/pwbot.html
> >
> >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64
2024-09-05 0:03 ` Andrii Nakryiko
@ 2024-09-05 2:26 ` Pu Lehui
0 siblings, 0 replies; 16+ messages in thread
From: Pu Lehui @ 2024-09-05 2:26 UTC (permalink / raw)
To: Andrii Nakryiko, patchwork-bot+netdevbpf
Cc: bpf, linux-riscv, Networking, Andrii Nakryiko,
Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, john fastabend, KP Singh, Hao Luo, Jiri Olsa,
Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On 2024/9/5 8:03, Andrii Nakryiko wrote:
> On Wed, Sep 4, 2024 at 3:44 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>>
>>
>> On Wed, Sep 4, 2024 at 1:30 PM <patchwork-bot+netdevbpf@kernel.org> wrote:
>>>
>>> Hello:
>>>
>>> This series was applied to bpf/bpf-next.git (master)
>>> by Andrii Nakryiko <andrii@kernel.org>:
>>>
>>> On Sat, 31 Aug 2024 04:19:30 +0000 you wrote:
>>>> On RV64, as Ilya mentioned before [0], the first syscall parameter should be
>>>> accessed through orig_a0 (see arch/riscv64/include/asm/syscall.h),
>>>> otherwise it will cause selftests like bpf_syscall_macro, vmlinux,
>>>> test_lsm, etc. to fail on RV64.
>>>>
>>>> Link: https://lore.kernel.org/bpf/20220209021745.2215452-1-iii@linux.ibm.com [0]
>>>>
>>>> [...]
>>>
>>> Here is the summary with links:
>>> - [bpf-next,v3,1/4] libbpf: Access first syscall argument with CO-RE direct read on s390
>>> https://git.kernel.org/bpf/bpf-next/c/65ee11d9c822
>>> - [bpf-next,v3,2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
>>> https://git.kernel.org/bpf/bpf-next/c/ebd8ad474888
>>> - [bpf-next,v3,3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64
>>> https://git.kernel.org/bpf/bpf-next/c/3a913c4d62e1
>>> - [bpf-next,v3,4/4] libbpf: Fix accessing first syscall argument on RV64
>>> https://git.kernel.org/bpf/bpf-next/c/13143c5816bc
>>>
>>
>> Ok, I had to "unapply" these patches, as s390x selftest (bpf_syscall_macro) started failing (arm64 is fine, so I think it's probably due to patch #3 that changes selftests itself).
>>
>> Pu, can you please take a look at that (e.g., see [0])? It's a bit strange, as originally no error was reported, so not sure what changed. Please also see the things I was fixing up while applying, so I don't have to do it again :)
>>
>> [0] https://github.com/kernel-patches/bpf/actions/runs/10709024755/job/29693056550#step:5:8746
>
> Oh, I figured it out! That tmp is there for a reason! We
> bpf_probe_read_kernel() 8 bytes, but syscall's 1st argument itself is
> 4 byte long, so we need to cast u64 to u32. And s390x being the big
> endian architecture detected this problem, while for arm64 we were
> lucky.
>
> So never mind, I'll apply your patches with fix ups in the
> bpf_tracing.h header, but I won't touch patch #3.
Sorry for couldn't reply in time due to jet lag. Glad the issue didn't
block in the end.😄
>
>>
>>> You are awesome, thank you!
>>> --
>>> Deet-doot-dot, I am a bot.
>>> https://korg.docs.kernel.org/patchwork/pwbot.html
>>>
>>>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-09-04 20:06 ` Andrii Nakryiko
@ 2024-09-05 2:39 ` Xu Kuohai
0 siblings, 0 replies; 16+ messages in thread
From: Xu Kuohai @ 2024-09-05 2:39 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Pu Lehui, bpf, linux-riscv, netdev, Andrii Nakryiko,
Björn Töpel, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Puranjay Mohan, Palmer Dabbelt, Pu Lehui
On 9/5/2024 4:06 AM, Andrii Nakryiko wrote:
> On Sat, Aug 31, 2024 at 12:57 AM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>>
>> On 8/31/2024 3:26 PM, Xu Kuohai wrote:
>>> On 8/31/2024 12:19 PM, Pu Lehui wrote:
>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>
>>>> Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
>>>> SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
>>>> into account the read pt_regs comes directly from the context, let's use
>>>> CO-RE direct read to access the first system call argument.
>>>>
>>>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>>> ---
>>>> tools/lib/bpf/bpf_tracing.h | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
>>>> index e7d9382efeb3..051c408e6aed 100644
>>>> --- a/tools/lib/bpf/bpf_tracing.h
>>>> +++ b/tools/lib/bpf/bpf_tracing.h
>>>> @@ -222,7 +222,7 @@ struct pt_regs___s390 {
>>>> struct pt_regs___arm64 {
>>>> unsigned long orig_x0;
>>>> -};
>>>> +} __attribute__((preserve_access_index));
>>>> /* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
>>>> #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
>>>> @@ -241,7 +241,7 @@ struct pt_regs___arm64 {
>>>> #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
>>>> #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
>>>> #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
>>>> -#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
>>>> +#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
>>>> #define PT_REGS_PARM1_CORE_SYSCALL(x) \
>>>> BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
>>>
>>> Cool!
>>>
>>> Acked-by: Xu Kuohai <xukuohai@huawei.com>
>>>
>>>
>>
>> Wait, it breaks the following test:
>>
>
> You mean, *if you change the existing test like below*, it will break,
> right? And that's expected, because arm64 has
> ARCH_HAS_SYSCALL_WRAPPER, which means syscall pt_regs are actually not
> the kprobe's ctx, so you can't directly access it. Which is why we
> have PT_REGS_PARM1_CORE_SYSCALL() variants.
>
> See how BPF_KSYSCALL macro is implemented, there are two cases:
> ___bpf_syswap_args(), which uses BPF_CORE_READ()-based macros to fetch
> arguments, and ___bpf_syscall_args() which uses direct ctx reads.
>
Got it, thanks for the explanation.
>
>> --- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
>> +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
>> @@ -43,7 +43,7 @@ int BPF_KPROBE(handle_sys_prctl)
>>
>> /* test for PT_REGS_PARM */
>>
>> - bpf_probe_read_kernel(&tmp, sizeof(tmp), &PT_REGS_PARM1_SYSCALL(real_regs));
>> + tmp = PT_REGS_PARM1_SYSCALL(real_regs);
>> arg1 = tmp;
>> bpf_probe_read_kernel(&arg2, sizeof(arg2), &PT_REGS_PARM2_SYSCALL(real_regs));
>> bpf_probe_read_kernel(&arg3, sizeof(arg3), &PT_REGS_PARM3_SYSCALL(real_regs));
>>
>> Failed with verifier rejection:
>>
>> 0: R1=ctx() R10=fp0
>> ; int BPF_KPROBE(handle_sys_prctl) @ bpf_syscall_macro.c:33
>> 0: (bf) r6 = r1 ; R1=ctx() R6_w=ctx()
>> ; pid_t pid = bpf_get_current_pid_tgid() >> 32; @ bpf_syscall_macro.c:36
>> 1: (85) call bpf_get_current_pid_tgid#14 ; R0_w=scalar()
>> ; if (pid != filter_pid) @ bpf_syscall_macro.c:39
>> 2: (18) r1 = 0xffff800082e0e000 ; R1_w=map_value(map=bpf_sysc.rodata,ks=4,vs=4)
>> 4: (61) r1 = *(u32 *)(r1 +0) ; R1_w=607
>> ; pid_t pid = bpf_get_current_pid_tgid() >> 32; @ bpf_syscall_macro.c:36
>> 5: (77) r0 >>= 32 ; R0_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
>> ; if (pid != filter_pid) @ bpf_syscall_macro.c:39
>> 6: (5e) if w1 != w0 goto pc+98 ; R0_w=607 R1_w=607
>> ; real_regs = PT_REGS_SYSCALL_REGS(ctx); @ bpf_syscall_macro.c:42
>> 7: (79) r8 = *(u64 *)(r6 +0) ; R6_w=ctx() R8_w=scalar()
>> ; tmp = PT_REGS_PARM1_SYSCALL(real_regs); @ bpf_syscall_macro.c:46
>> 8: (79) r1 = *(u64 *)(r8 +272)
>> R8 invalid mem access 'scalar'
>> processed 8 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
2024-09-04 20:21 ` Andrii Nakryiko
@ 2024-09-05 6:42 ` Pu Lehui
0 siblings, 0 replies; 16+ messages in thread
From: Pu Lehui @ 2024-09-05 6:42 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, linux-riscv, netdev, Andrii Nakryiko, Björn Töpel,
Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Puranjay Mohan,
Palmer Dabbelt, Pu Lehui
On 2024/9/5 4:21, Andrii Nakryiko wrote:
> On Fri, Aug 30, 2024 at 9:17 PM Pu Lehui <pulehui@huaweicloud.com> wrote:
>>
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> Currently PT_REGS_PARM1 SYSCALL(x) is consistent with PT_REGS_PARM1_CORE
>> SYSCALL(x), which will introduce the overhead of BPF_CORE_READ(), taking
>> into account the read pt_regs comes directly from the context, let's use
>> CO-RE direct read to access the first system call argument.
>>
>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> ---
>> tools/lib/bpf/bpf_tracing.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
>> index e7d9382efeb3..051c408e6aed 100644
>> --- a/tools/lib/bpf/bpf_tracing.h
>> +++ b/tools/lib/bpf/bpf_tracing.h
>> @@ -222,7 +222,7 @@ struct pt_regs___s390 {
>>
>> struct pt_regs___arm64 {
>> unsigned long orig_x0;
>> -};
>> +} __attribute__((preserve_access_index));
>>
>> /* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
>> #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
>> @@ -241,7 +241,7 @@ struct pt_regs___arm64 {
>> #define __PT_PARM4_SYSCALL_REG __PT_PARM4_REG
>> #define __PT_PARM5_SYSCALL_REG __PT_PARM5_REG
>> #define __PT_PARM6_SYSCALL_REG __PT_PARM6_REG
>> -#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1_CORE_SYSCALL(x)
>> +#define PT_REGS_PARM1_SYSCALL(x) (((const struct pt_regs___arm64 *)(x))->orig_x0)
>
> It would probably be best (for consistency) to stick to using
> __PTR_PARM1_SYSCALL_REG instead of hard-coding orig_x0 here, no? I'll
> fix it up while applying. Same for patch #1 and #4.
>
> It would be great if you can double-check that final patches in
> bpf-next/master compile and work well for arm64, s390x, and RV64 (as I
> can't really test that much locally).
I check that locally with cross-platform vmtest on RV64, it looks good:
Summary: 569/3944 PASSED, 104 SKIPPED, 0 FAILED
and BPF CI meet happy on arm64, s390x.
>
>
>
>> #define PT_REGS_PARM1_CORE_SYSCALL(x) \
>> BPF_CORE_READ((const struct pt_regs___arm64 *)(x), __PT_PARM1_SYSCALL_REG)
>>
>> --
>> 2.34.1
>>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
` (4 preceding siblings ...)
2024-09-04 20:30 ` [PATCH bpf-next v3 0/4] " patchwork-bot+netdevbpf
@ 2024-10-01 11:35 ` patchwork-bot+linux-riscv
5 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+linux-riscv @ 2024-10-01 11:35 UTC (permalink / raw)
To: Pu Lehui
Cc: linux-riscv, bpf, netdev, andrii, bjorn, iii, ast, daniel,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, puranjay, palmer, pulehui
Hello:
This series was applied to riscv/linux.git (fixes)
by Andrii Nakryiko <andrii@kernel.org>:
On Sat, 31 Aug 2024 04:19:30 +0000 you wrote:
> On RV64, as Ilya mentioned before [0], the first syscall parameter should be
> accessed through orig_a0 (see arch/riscv64/include/asm/syscall.h),
> otherwise it will cause selftests like bpf_syscall_macro, vmlinux,
> test_lsm, etc. to fail on RV64.
>
> Link: https://lore.kernel.org/bpf/20220209021745.2215452-1-iii@linux.ibm.com [0]
>
> [...]
Here is the summary with links:
- [bpf-next,v3,1/4] libbpf: Access first syscall argument with CO-RE direct read on s390
https://git.kernel.org/riscv/c/e4db2a821b6c
- [bpf-next,v3,2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64
https://git.kernel.org/riscv/c/9ab94078e868
- [bpf-next,v3,3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64
https://git.kernel.org/riscv/c/4a4c4c0d0a42
- [bpf-next,v3,4/4] libbpf: Fix accessing first syscall argument on RV64
https://git.kernel.org/riscv/c/99857422338b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-10-01 11:50 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-31 4:19 [PATCH bpf-next v3 0/4] Fix accessing first syscall argument on RV64 Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 1/4] libbpf: Access first syscall argument with CO-RE direct read on s390 Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 2/4] libbpf: Access first syscall argument with CO-RE direct read on arm64 Pu Lehui
2024-08-31 7:26 ` Xu Kuohai
2024-08-31 7:57 ` Xu Kuohai
2024-09-04 20:06 ` Andrii Nakryiko
2024-09-05 2:39 ` Xu Kuohai
2024-09-04 20:21 ` Andrii Nakryiko
2024-09-05 6:42 ` Pu Lehui
2024-08-31 4:19 ` [PATCH bpf-next v3 3/4] selftests/bpf: Enable test_bpf_syscall_macro:syscall_arg1 on s390 and arm64 Pu Lehui
2024-09-04 20:21 ` Andrii Nakryiko
2024-08-31 4:19 ` [PATCH bpf-next v3 4/4] libbpf: Fix accessing first syscall argument on RV64 Pu Lehui
2024-09-04 20:30 ` [PATCH bpf-next v3 0/4] " patchwork-bot+netdevbpf
[not found] ` <CAEf4BzayV1ihbfSg4fv0AqSazVycXqCJp4dTq1pwRt5hmx7X4g@mail.gmail.com>
2024-09-05 0:03 ` Andrii Nakryiko
2024-09-05 2:26 ` Pu Lehui
2024-10-01 11:35 ` patchwork-bot+linux-riscv
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox