* [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
@ 2019-07-09 15:18 Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(ARCH) Ilya Leoshkevich
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2019-07-09 15:18 UTC (permalink / raw)
To: bpf, netdev; +Cc: sdf, ys114321, davem, ast, daniel, Ilya Leoshkevich
Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
This patch series consists of three preparatory commits, which make it
possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
Since the last time, I've tested it with x86_64-linux-gnu-,
aarch64-linux-gnu-, arm-linux-gnueabihf-, mips64el-linux-gnuabi64-,
powerpc64le-linux-gnu-, s390x-linux-gnu- and sparc64-linux-gnu-
compilers, and found that I also need to add arm64 support.
Like s390, arm64 exports user_pt_regs instead of struct pt_regs to
userspace.
I've also made fixes for a few unrelated build problems, which I will
post separately.
v1->v2: Split into multiple patches.
v2->v3: Added arm64 support.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(ARCH)
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
@ 2019-07-09 15:18 ` Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2019-07-09 15:18 UTC (permalink / raw)
To: bpf, netdev; +Cc: sdf, ys114321, davem, ast, daniel, Ilya Leoshkevich
This opens up the possibility of accessing registers in an
arch-independent way.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 2620406a53ec..59d89d5aa05e 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+include ../../../scripts/Makefile.arch
LIBDIR := ../../../lib
BPFDIR := $(LIBDIR)/bpf
@@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
$(CLANG_SYS_INCLUDES) \
- -Wno-compare-distinct-pointer-types
+ -Wno-compare-distinct-pointer-types \
+ -D__TARGET_ARCH_$(ARCH)
$(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
$(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(ARCH) Ilya Leoshkevich
@ 2019-07-09 15:18 ` Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2019-07-09 15:18 UTC (permalink / raw)
To: bpf, netdev; +Cc: sdf, ys114321, davem, ast, daniel, Ilya Leoshkevich
Also check for __s390__ instead of __s390x__, just in case bpf_helpers.h
is ever used by 32-bit userspace.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5a3d92c8bec8..73071a94769a 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -315,8 +315,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#if defined(__TARGET_ARCH_x86)
#define bpf_target_x86
#define bpf_target_defined
-#elif defined(__TARGET_ARCH_s930x)
- #define bpf_target_s930x
+#elif defined(__TARGET_ARCH_s390)
+ #define bpf_target_s390
#define bpf_target_defined
#elif defined(__TARGET_ARCH_arm)
#define bpf_target_arm
@@ -341,8 +341,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#ifndef bpf_target_defined
#if defined(__x86_64__)
#define bpf_target_x86
-#elif defined(__s390x__)
- #define bpf_target_s930x
+#elif defined(__s390__)
+ #define bpf_target_s390
#elif defined(__arm__)
#define bpf_target_arm
#elif defined(__aarch64__)
@@ -369,7 +369,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#define PT_REGS_SP(x) ((x)->sp)
#define PT_REGS_IP(x) ((x)->ip)
-#elif defined(bpf_target_s390x)
+#elif defined(bpf_target_s390)
#define PT_REGS_PARM1(x) ((x)->gprs[2])
#define PT_REGS_PARM2(x) ((x)->gprs[3])
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(ARCH) Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
@ 2019-07-09 15:18 ` Ilya Leoshkevich
2019-07-09 17:48 ` Andrii Nakryiko
2019-07-09 15:18 ` [PATCH v3 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Ilya Leoshkevich @ 2019-07-09 15:18 UTC (permalink / raw)
To: bpf, netdev; +Cc: sdf, ys114321, davem, ast, daniel, Ilya Leoshkevich
Right now, on certain architectures, these macros are usable only with
kernel headers. This patch makes it possible to use them with userspace
headers and, as a consequence, not only in BPF samples, but also in BPF
selftests.
On s390, provide the forward declaration of struct pt_regs and cast it
to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
of the full struct pt_regs, s390 exposes only its first member
user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
(in selftests) and kernel (in samples) headers. It was added in commit
466698e654e8 ("s390/bpf: correct broken uapi for
BPF_PROG_TYPE_PERF_EVENT program type").
Ditto on arm64.
On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and
arm64, x86 provides struct pt_regs to both userspace and kernel, however,
with different member names.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 61 +++++++++++++++--------
1 file changed, 41 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 73071a94769a..212ec564e5c3 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -358,6 +358,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#if defined(bpf_target_x86)
+#ifdef __KERNEL__
#define PT_REGS_PARM1(x) ((x)->di)
#define PT_REGS_PARM2(x) ((x)->si)
#define PT_REGS_PARM3(x) ((x)->dx)
@@ -368,19 +369,35 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#define PT_REGS_RC(x) ((x)->ax)
#define PT_REGS_SP(x) ((x)->sp)
#define PT_REGS_IP(x) ((x)->ip)
+#else
+#define PT_REGS_PARM1(x) ((x)->rdi)
+#define PT_REGS_PARM2(x) ((x)->rsi)
+#define PT_REGS_PARM3(x) ((x)->rdx)
+#define PT_REGS_PARM4(x) ((x)->rcx)
+#define PT_REGS_PARM5(x) ((x)->r8)
+#define PT_REGS_RET(x) ((x)->rsp)
+#define PT_REGS_FP(x) ((x)->rbp)
+#define PT_REGS_RC(x) ((x)->rax)
+#define PT_REGS_SP(x) ((x)->rsp)
+#define PT_REGS_IP(x) ((x)->rip)
+#endif
#elif defined(bpf_target_s390)
-#define PT_REGS_PARM1(x) ((x)->gprs[2])
-#define PT_REGS_PARM2(x) ((x)->gprs[3])
-#define PT_REGS_PARM3(x) ((x)->gprs[4])
-#define PT_REGS_PARM4(x) ((x)->gprs[5])
-#define PT_REGS_PARM5(x) ((x)->gprs[6])
-#define PT_REGS_RET(x) ((x)->gprs[14])
-#define PT_REGS_FP(x) ((x)->gprs[11]) /* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_RC(x) ((x)->gprs[2])
-#define PT_REGS_SP(x) ((x)->gprs[15])
-#define PT_REGS_IP(x) ((x)->psw.addr)
+/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
+struct pt_regs;
+#define PT_REGS_S390 const volatile user_pt_regs
+#define PT_REGS_PARM1(x) (((PT_REGS_S390 *)(x))->gprs[2])
+#define PT_REGS_PARM2(x) (((PT_REGS_S390 *)(x))->gprs[3])
+#define PT_REGS_PARM3(x) (((PT_REGS_S390 *)(x))->gprs[4])
+#define PT_REGS_PARM4(x) (((PT_REGS_S390 *)(x))->gprs[5])
+#define PT_REGS_PARM5(x) (((PT_REGS_S390 *)(x))->gprs[6])
+#define PT_REGS_RET(x) (((PT_REGS_S390 *)(x))->gprs[14])
+/* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_FP(x) (((PT_REGS_S390 *)(x))->gprs[11])
+#define PT_REGS_RC(x) (((PT_REGS_S390 *)(x))->gprs[2])
+#define PT_REGS_SP(x) (((PT_REGS_S390 *)(x))->gprs[15])
+#define PT_REGS_IP(x) (((PT_REGS_S390 *)(x))->psw.addr)
#elif defined(bpf_target_arm)
@@ -397,16 +414,20 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
#elif defined(bpf_target_arm64)
-#define PT_REGS_PARM1(x) ((x)->regs[0])
-#define PT_REGS_PARM2(x) ((x)->regs[1])
-#define PT_REGS_PARM3(x) ((x)->regs[2])
-#define PT_REGS_PARM4(x) ((x)->regs[3])
-#define PT_REGS_PARM5(x) ((x)->regs[4])
-#define PT_REGS_RET(x) ((x)->regs[30])
-#define PT_REGS_FP(x) ((x)->regs[29]) /* Works only with CONFIG_FRAME_POINTER */
-#define PT_REGS_RC(x) ((x)->regs[0])
-#define PT_REGS_SP(x) ((x)->sp)
-#define PT_REGS_IP(x) ((x)->pc)
+/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
+struct pt_regs;
+#define PT_REGS_ARM64 const volatile struct user_pt_regs
+#define PT_REGS_PARM1(x) (((PT_REGS_ARM64 *)(x))->regs[0])
+#define PT_REGS_PARM2(x) (((PT_REGS_ARM64 *)(x))->regs[1])
+#define PT_REGS_PARM3(x) (((PT_REGS_ARM64 *)(x))->regs[2])
+#define PT_REGS_PARM4(x) (((PT_REGS_ARM64 *)(x))->regs[3])
+#define PT_REGS_PARM5(x) (((PT_REGS_ARM64 *)(x))->regs[4])
+#define PT_REGS_RET(x) (((PT_REGS_ARM64 *)(x))->regs[30])
+/* Works only with CONFIG_FRAME_POINTER */
+#define PT_REGS_FP(x) (((PT_REGS_ARM64 *)(x))->regs[29])
+#define PT_REGS_RC(x) (((PT_REGS_ARM64 *)(x))->regs[0])
+#define PT_REGS_SP(x) (((PT_REGS_ARM64 *)(x))->sp)
+#define PT_REGS_IP(x) (((PT_REGS_ARM64 *)(x))->pc)
#elif defined(bpf_target_mips)
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
` (2 preceding siblings ...)
2019-07-09 15:18 ` [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
@ 2019-07-09 15:18 ` Ilya Leoshkevich
2019-07-09 16:07 ` [PATCH v3 bpf-next 0/4] " Stanislav Fomichev
2019-07-09 17:50 ` Andrii Nakryiko
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2019-07-09 15:18 UTC (permalink / raw)
To: bpf, netdev; +Cc: sdf, ys114321, davem, ast, daniel, Ilya Leoshkevich
Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/progs/loop1.c | 2 +-
tools/testing/selftests/bpf/progs/loop2.c | 2 +-
tools/testing/selftests/bpf/progs/loop3.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c
index dea395af9ea9..7cdb7f878310 100644
--- a/tools/testing/selftests/bpf/progs/loop1.c
+++ b/tools/testing/selftests/bpf/progs/loop1.c
@@ -18,7 +18,7 @@ int nested_loops(volatile struct pt_regs* ctx)
for (j = 0; j < 300; j++)
for (i = 0; i < j; i++) {
if (j & 1)
- m = ctx->rax;
+ m = PT_REGS_RC(ctx);
else
m = j;
sum += i * m;
diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c
index 0637bd8e8bcf..9b2f808a2863 100644
--- a/tools/testing/selftests/bpf/progs/loop2.c
+++ b/tools/testing/selftests/bpf/progs/loop2.c
@@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx)
int i = 0;
while (true) {
- if (ctx->rax & 1)
+ if (PT_REGS_RC(ctx) & 1)
i += 3;
else
i += 7;
diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c
index 30a0f6cba080..d727657d51e2 100644
--- a/tools/testing/selftests/bpf/progs/loop3.c
+++ b/tools/testing/selftests/bpf/progs/loop3.c
@@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx)
__u64 i = 0, sum = 0;
do {
i++;
- sum += ctx->rax;
+ sum += PT_REGS_RC(ctx);
} while (i < 0x100000000ULL);
return sum;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
` (3 preceding siblings ...)
2019-07-09 15:18 ` [PATCH v3 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
@ 2019-07-09 16:07 ` Stanislav Fomichev
2019-07-09 17:50 ` Andrii Nakryiko
5 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2019-07-09 16:07 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev, ys114321, davem, ast, daniel
On 07/09, Ilya Leoshkevich wrote:
> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>
> This patch series consists of three preparatory commits, which make it
> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
>
> Since the last time, I've tested it with x86_64-linux-gnu-,
> aarch64-linux-gnu-, arm-linux-gnueabihf-, mips64el-linux-gnuabi64-,
> powerpc64le-linux-gnu-, s390x-linux-gnu- and sparc64-linux-gnu-
> compilers, and found that I also need to add arm64 support.
>
> Like s390, arm64 exports user_pt_regs instead of struct pt_regs to
> userspace.
>
> I've also made fixes for a few unrelated build problems, which I will
> post separately.
>
> v1->v2: Split into multiple patches.
> v2->v3: Added arm64 support.
For the whole series:
Reviewed-by: Stanislav Fomichev <sdf@google.com>
This should probably go to bpf, not bpf-next since it fixes the
existing compilation problem.
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace
2019-07-09 15:18 ` [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
@ 2019-07-09 17:48 ` Andrii Nakryiko
2019-07-10 11:47 ` Ilya Leoshkevich
0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2019-07-09 17:48 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Stanislav Fomichev, Y Song, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
On Tue, Jul 9, 2019 at 8:19 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Right now, on certain architectures, these macros are usable only with
> kernel headers. This patch makes it possible to use them with userspace
> headers and, as a consequence, not only in BPF samples, but also in BPF
> selftests.
>
> On s390, provide the forward declaration of struct pt_regs and cast it
> to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
> of the full struct pt_regs, s390 exposes only its first member
> user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
> (in selftests) and kernel (in samples) headers. It was added in commit
> 466698e654e8 ("s390/bpf: correct broken uapi for
> BPF_PROG_TYPE_PERF_EVENT program type").
>
> Ditto on arm64.
>
> On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and
> arm64, x86 provides struct pt_regs to both userspace and kernel, however,
> with different member names.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
Just curious, what did you use as a reference for which register
corresponds to which PARM, RET, etc for different archs? I've tried to
look it up the other day, and it wasn't as straightforward to find as
I hoped for, so maybe I'm missing something obvious.
> tools/testing/selftests/bpf/bpf_helpers.h | 61 +++++++++++++++--------
> 1 file changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 73071a94769a..212ec564e5c3 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -358,6 +358,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
>
> #if defined(bpf_target_x86)
>
> +#ifdef __KERNEL__
> #define PT_REGS_PARM1(x) ((x)->di)
> #define PT_REGS_PARM2(x) ((x)->si)
> #define PT_REGS_PARM3(x) ((x)->dx)
> @@ -368,19 +369,35 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #define PT_REGS_RC(x) ((x)->ax)
> #define PT_REGS_SP(x) ((x)->sp)
> #define PT_REGS_IP(x) ((x)->ip)
> +#else
> +#define PT_REGS_PARM1(x) ((x)->rdi)
> +#define PT_REGS_PARM2(x) ((x)->rsi)
> +#define PT_REGS_PARM3(x) ((x)->rdx)
> +#define PT_REGS_PARM4(x) ((x)->rcx)
> +#define PT_REGS_PARM5(x) ((x)->r8)
> +#define PT_REGS_RET(x) ((x)->rsp)
> +#define PT_REGS_FP(x) ((x)->rbp)
> +#define PT_REGS_RC(x) ((x)->rax)
> +#define PT_REGS_SP(x) ((x)->rsp)
> +#define PT_REGS_IP(x) ((x)->rip)
Will this also work for 32-bit x86?
> +#endif
<snip>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
` (4 preceding siblings ...)
2019-07-09 16:07 ` [PATCH v3 bpf-next 0/4] " Stanislav Fomichev
@ 2019-07-09 17:50 ` Andrii Nakryiko
5 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-07-09 17:50 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Stanislav Fomichev, Y Song, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
On Tue, Jul 9, 2019 at 8:18 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>
> This patch series consists of three preparatory commits, which make it
> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
>
> Since the last time, I've tested it with x86_64-linux-gnu-,
> aarch64-linux-gnu-, arm-linux-gnueabihf-, mips64el-linux-gnuabi64-,
> powerpc64le-linux-gnu-, s390x-linux-gnu- and sparc64-linux-gnu-
> compilers, and found that I also need to add arm64 support.
>
> Like s390, arm64 exports user_pt_regs instead of struct pt_regs to
> userspace.
>
> I've also made fixes for a few unrelated build problems, which I will
> post separately.
>
> v1->v2: Split into multiple patches.
> v2->v3: Added arm64 support.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>
>
Acked-by: Andrii Nakryiko <andriin@fb.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace
2019-07-09 17:48 ` Andrii Nakryiko
@ 2019-07-10 11:47 ` Ilya Leoshkevich
2019-07-10 17:14 ` Andrii Nakryiko
0 siblings, 1 reply; 10+ messages in thread
From: Ilya Leoshkevich @ 2019-07-10 11:47 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Stanislav Fomichev, Y Song, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Adrian Ratiu, david.daney
> Am 09.07.2019 um 19:48 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Tue, Jul 9, 2019 at 8:19 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> Right now, on certain architectures, these macros are usable only with
>> kernel headers. This patch makes it possible to use them with userspace
>> headers and, as a consequence, not only in BPF samples, but also in BPF
>> selftests.
>>
>> On s390, provide the forward declaration of struct pt_regs and cast it
>> to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
>> of the full struct pt_regs, s390 exposes only its first member
>> user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
>> (in selftests) and kernel (in samples) headers. It was added in commit
>> 466698e654e8 ("s390/bpf: correct broken uapi for
>> BPF_PROG_TYPE_PERF_EVENT program type").
>>
>> Ditto on arm64.
>>
>> On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and
>> arm64, x86 provides struct pt_regs to both userspace and kernel, however,
>> with different member names.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>
> Just curious, what did you use as a reference for which register
> corresponds to which PARM, RET, etc for different archs? I've tried to
> look it up the other day, and it wasn't as straightforward to find as
> I hoped for, so maybe I'm missing something obvious.
For this particular change I did not have to look it up, because it all
was already in the code, I just needed to adapt it to userspace headers.
Normally I would google for „abi supplement“ to find this information.
A lazy way would be to simply ask the (cross-)compiler:
cat <<HERE | aarch64-linux-gnu-gcc -x c -O3 -S - -o -
int f(int a, int b, int c, int d, int e, int f, int g, int h, int i, int j);
int g() { return -f(1, 2, 3, 4, 5, 6, 7, 8, 9, 10); }
HERE
I’ve just double checked the supported arches, and noticed that:
#define PT_REGS_PARM5(x) ((x)->uregs[4])
for bpf_target_arm (arm-linux-gnueabihf) looks wrong:
the 5th parameter should be passed on stack. This observation matches
[1].
#define PT_REGS_RC(x) ((x)->regs[1])
for bpf_target_mips (mips64el-linux-gnuabi64) also looks wrong:
the return value should be in register 2. This observation matches [2].
Since I’m not an expert on those architectures, my conclusions could be
incorrect (e.g. becase a different ABI is normally used in practice).
Adrian and David, could you please correct me if I’m wrong?
[1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf
[2] ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/psABI_mips3.0.pdf
>> tools/testing/selftests/bpf/bpf_helpers.h | 61 +++++++++++++++--------
>> 1 file changed, 41 insertions(+), 20 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
>> index 73071a94769a..212ec564e5c3 100644
>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
>> @@ -358,6 +358,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
>>
>> #if defined(bpf_target_x86)
>>
>> +#ifdef __KERNEL__
>> #define PT_REGS_PARM1(x) ((x)->di)
>> #define PT_REGS_PARM2(x) ((x)->si)
>> #define PT_REGS_PARM3(x) ((x)->dx)
>> @@ -368,19 +369,35 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
>> #define PT_REGS_RC(x) ((x)->ax)
>> #define PT_REGS_SP(x) ((x)->sp)
>> #define PT_REGS_IP(x) ((x)->ip)
>> +#else
>> +#define PT_REGS_PARM1(x) ((x)->rdi)
>> +#define PT_REGS_PARM2(x) ((x)->rsi)
>> +#define PT_REGS_PARM3(x) ((x)->rdx)
>> +#define PT_REGS_PARM4(x) ((x)->rcx)
>> +#define PT_REGS_PARM5(x) ((x)->r8)
>> +#define PT_REGS_RET(x) ((x)->rsp)
>> +#define PT_REGS_FP(x) ((x)->rbp)
>> +#define PT_REGS_RC(x) ((x)->rax)
>> +#define PT_REGS_SP(x) ((x)->rsp)
>> +#define PT_REGS_IP(x) ((x)->rip)
>
> Will this also work for 32-bit x86?
Thanks, this is a good catch: this builds, but makes 64-bit accesses, as if it used the 64-bit
variant of pt_regs. I will fix this.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace
2019-07-10 11:47 ` Ilya Leoshkevich
@ 2019-07-10 17:14 ` Andrii Nakryiko
0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-07-10 17:14 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: bpf, Networking, Stanislav Fomichev, Y Song, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Adrian Ratiu, david.daney
On Wed, Jul 10, 2019 at 4:47 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> > Am 09.07.2019 um 19:48 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
> >
> > On Tue, Jul 9, 2019 at 8:19 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >>
> >> Right now, on certain architectures, these macros are usable only with
> >> kernel headers. This patch makes it possible to use them with userspace
> >> headers and, as a consequence, not only in BPF samples, but also in BPF
> >> selftests.
> >>
> >> On s390, provide the forward declaration of struct pt_regs and cast it
> >> to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
> >> of the full struct pt_regs, s390 exposes only its first member
> >> user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
> >> (in selftests) and kernel (in samples) headers. It was added in commit
> >> 466698e654e8 ("s390/bpf: correct broken uapi for
> >> BPF_PROG_TYPE_PERF_EVENT program type").
> >>
> >> Ditto on arm64.
> >>
> >> On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and
> >> arm64, x86 provides struct pt_regs to both userspace and kernel, however,
> >> with different member names.
> >>
> >> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> >> ---
> >
> > Just curious, what did you use as a reference for which register
> > corresponds to which PARM, RET, etc for different archs? I've tried to
> > look it up the other day, and it wasn't as straightforward to find as
> > I hoped for, so maybe I'm missing something obvious.
>
> For this particular change I did not have to look it up, because it all
> was already in the code, I just needed to adapt it to userspace headers.
> Normally I would google for „abi supplement“ to find this information.
> A lazy way would be to simply ask the (cross-)compiler:
>
> cat <<HERE | aarch64-linux-gnu-gcc -x c -O3 -S - -o -
> int f(int a, int b, int c, int d, int e, int f, int g, int h, int i, int j);
> int g() { return -f(1, 2, 3, 4, 5, 6, 7, 8, 9, 10); }
> HERE
Thanks for this trick! :)
>
> I’ve just double checked the supported arches, and noticed that:
>
> #define PT_REGS_PARM5(x) ((x)->uregs[4])
> for bpf_target_arm (arm-linux-gnueabihf) looks wrong:
> the 5th parameter should be passed on stack. This observation matches
> [1].
>
> #define PT_REGS_RC(x) ((x)->regs[1])
> for bpf_target_mips (mips64el-linux-gnuabi64) also looks wrong:
> the return value should be in register 2. This observation matches [2].
Now I'm glad I asked :)
>
> Since I’m not an expert on those architectures, my conclusions could be
> incorrect (e.g. becase a different ABI is normally used in practice).
> Adrian and David, could you please correct me if I’m wrong?
>
> [1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf
> [2] ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/psABI_mips3.0.pdf
>
> >> tools/testing/selftests/bpf/bpf_helpers.h | 61 +++++++++++++++--------
> >> 1 file changed, 41 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> >> index 73071a94769a..212ec564e5c3 100644
> >> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> >> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> >> @@ -358,6 +358,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> >>
> >> #if defined(bpf_target_x86)
> >>
> >> +#ifdef __KERNEL__
> >> #define PT_REGS_PARM1(x) ((x)->di)
> >> #define PT_REGS_PARM2(x) ((x)->si)
> >> #define PT_REGS_PARM3(x) ((x)->dx)
> >> @@ -368,19 +369,35 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> >> #define PT_REGS_RC(x) ((x)->ax)
> >> #define PT_REGS_SP(x) ((x)->sp)
> >> #define PT_REGS_IP(x) ((x)->ip)
> >> +#else
> >> +#define PT_REGS_PARM1(x) ((x)->rdi)
> >> +#define PT_REGS_PARM2(x) ((x)->rsi)
> >> +#define PT_REGS_PARM3(x) ((x)->rdx)
> >> +#define PT_REGS_PARM4(x) ((x)->rcx)
> >> +#define PT_REGS_PARM5(x) ((x)->r8)
> >> +#define PT_REGS_RET(x) ((x)->rsp)
> >> +#define PT_REGS_FP(x) ((x)->rbp)
> >> +#define PT_REGS_RC(x) ((x)->rax)
> >> +#define PT_REGS_SP(x) ((x)->rsp)
> >> +#define PT_REGS_IP(x) ((x)->rip)
> >
> > Will this also work for 32-bit x86?
>
> Thanks, this is a good catch: this builds, but makes 64-bit accesses, as if it used the 64-bit
> variant of pt_regs. I will fix this.
Sounds good, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-10 17:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 15:18 [PATCH v3 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 1/4] selftests/bpf: compile progs with -D__TARGET_ARCH_$(ARCH) Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 2/4] selftests/bpf: fix s930 -> s390 typo Ilya Leoshkevich
2019-07-09 15:18 ` [PATCH v3 bpf-next 3/4] selftests/bpf: make PT_REGS_* work in userspace Ilya Leoshkevich
2019-07-09 17:48 ` Andrii Nakryiko
2019-07-10 11:47 ` Ilya Leoshkevich
2019-07-10 17:14 ` Andrii Nakryiko
2019-07-09 15:18 ` [PATCH v3 bpf-next 4/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390 Ilya Leoshkevich
2019-07-09 16:07 ` [PATCH v3 bpf-next 0/4] " Stanislav Fomichev
2019-07-09 17:50 ` Andrii Nakryiko
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).