All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] bpf: s390: add JIT support for multi-function programs
@ 2019-08-26 18:20 Yauheni Kaliuta
  2019-08-27 13:21 ` Ilya Leoshkevich
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-26 18:20 UTC (permalink / raw)
  To: bpf; +Cc: daniel, iii, jolsa

This adds support for bpf-to-bpf function calls in the s390 JIT
compiler. The JIT compiler converts the bpf call instructions to
native branch instructions. After a round of the usual passes, the
start addresses of the JITed images for the callee functions are
known. Finally, to fixup the branch target addresses, we need to
perform an extra pass.

Because of the address range in which JITed images are allocated on
s390, the offsets of the start addresses of these images from
__bpf_call_base are as large as 64 bits. So, for a function call,
the imm field of the instruction cannot be used to determine the
callee's address. Use bpf_jit_get_func_addr() helper instead.

The patch borrows a lot from:

8c11ea5ce13d bpf, arm64: fix getting subprog addr from aux for calls
e2c95a61656d bpf, ppc64: generalize fetching subprog into bpf_jit_get_func_addr
8484ce8306f9 bpf: powerpc64: add JIT support for multi-function programs

(including the commit message).

test_verifier (5.3-rc6):

without patch:
Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED

with patch:
Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 arch/s390/net/bpf_jit_comp.c | 63 +++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 11 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index e636728ab452..39329c20dcbb 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -502,7 +502,8 @@ static void bpf_jit_epilogue(struct bpf_jit *jit, u32 stack_depth)
  * NOTE: Use noinline because for gcov (-fprofile-arcs) gcc allocates a lot of
  * stack space for the large switch statement.
  */
-static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i)
+static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
+				 int i, bool extra_pass)
 {
 	struct bpf_insn *insn = &fp->insnsi[i];
 	int jmp_off, last, insn_count = 1;
@@ -1011,10 +1012,14 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 	 */
 	case BPF_JMP | BPF_CALL:
 	{
-		/*
-		 * b0 = (__bpf_call_base + imm)(b1, b2, b3, b4, b5)
-		 */
-		const u64 func = (u64)__bpf_call_base + imm;
+		u64 func;
+		bool func_addr_fixed;
+		int ret;
+
+		ret = bpf_jit_get_func_addr(fp, insn, extra_pass,
+					    &func, &func_addr_fixed);
+		if (ret < 0)
+			return ret;
 
 		REG_SET_SEEN(BPF_REG_5);
 		jit->seen |= SEEN_FUNC;
@@ -1281,7 +1286,7 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 /*
  * Compile eBPF program into s390x code
  */
-static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
+static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp, bool extra_pass)
 {
 	int i, insn_count;
 
@@ -1290,7 +1295,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
 
 	bpf_jit_prologue(jit, fp->aux->stack_depth);
 	for (i = 0; i < fp->len; i += insn_count) {
-		insn_count = bpf_jit_insn(jit, fp, i);
+		insn_count = bpf_jit_insn(jit, fp, i, extra_pass);
 		if (insn_count < 0)
 			return -1;
 		/* Next instruction address */
@@ -1309,6 +1314,12 @@ bool bpf_jit_needs_zext(void)
 	return true;
 }
 
+
+struct s390_jit_data {
+	struct bpf_binary_header *header;
+	struct bpf_jit ctx;
+};
+
 /*
  * Compile eBPF program "fp"
  */
@@ -1316,7 +1327,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 {
 	struct bpf_prog *tmp, *orig_fp = fp;
 	struct bpf_binary_header *header;
+	struct s390_jit_data *jit_data;
 	bool tmp_blinded = false;
+	bool extra_pass = false;
 	struct bpf_jit jit;
 	int pass;
 
@@ -1335,6 +1348,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = tmp;
 	}
 
+	jit_data = fp->aux->jit_data;
+	if (!jit_data) {
+		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
+		if (!jit_data) {
+			fp = orig_fp;
+			goto out;
+		}
+		fp->aux->jit_data = jit_data;
+	}
+	if (jit_data->ctx.addrs) {
+		jit = jit_data->ctx;
+		header = jit_data->header;
+		extra_pass = true;
+		goto skip_init_ctx;
+	}
+
 	memset(&jit, 0, sizeof(jit));
 	jit.addrs = kcalloc(fp->len + 1, sizeof(*jit.addrs), GFP_KERNEL);
 	if (jit.addrs == NULL) {
@@ -1347,7 +1376,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	 *   - 3:   Calculate program size and addrs arrray
 	 */
 	for (pass = 1; pass <= 3; pass++) {
-		if (bpf_jit_prog(&jit, fp)) {
+		if (bpf_jit_prog(&jit, fp, extra_pass)) {
 			fp = orig_fp;
 			goto free_addrs;
 		}
@@ -1359,12 +1388,14 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = orig_fp;
 		goto free_addrs;
 	}
+
 	header = bpf_jit_binary_alloc(jit.size, &jit.prg_buf, 2, jit_fill_hole);
 	if (!header) {
 		fp = orig_fp;
 		goto free_addrs;
 	}
-	if (bpf_jit_prog(&jit, fp)) {
+skip_init_ctx:
+	if (bpf_jit_prog(&jit, fp, extra_pass)) {
 		bpf_jit_binary_free(header);
 		fp = orig_fp;
 		goto free_addrs;
@@ -1373,12 +1404,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
 		print_fn_code(jit.prg_buf, jit.size_prg);
 	}
-	bpf_jit_binary_lock_ro(header);
+	if (!fp->is_func || extra_pass) {
+		bpf_jit_binary_lock_ro(header);
+	} else {
+		jit_data->header = header;
+		jit_data->ctx = jit;
+	}
 	fp->bpf_func = (void *) jit.prg_buf;
 	fp->jited = 1;
 	fp->jited_len = jit.size;
+
+	if (!fp->is_func || extra_pass) {
 free_addrs:
-	kfree(jit.addrs);
+		kfree(jit.addrs);
+		kfree(jit_data);
+		fp->aux->jit_data = NULL;
+	}
 out:
 	if (tmp_blinded)
 		bpf_jit_prog_release_other(fp, fp == orig_fp ?
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-26 18:20 [RFC PATCH] bpf: s390: add JIT support for multi-function programs Yauheni Kaliuta
@ 2019-08-27 13:21 ` Ilya Leoshkevich
  2019-08-27 13:46   ` Ilya Leoshkevich
  2019-08-27 14:25   ` Yauheni Kaliuta
  2019-08-27 14:53 ` [PATCH v2] " Yauheni Kaliuta
  2019-08-28 18:28 ` [PATCH v3] " Yauheni Kaliuta
  2 siblings, 2 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-27 13:21 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
> 
> test_verifier (5.3-rc6):
> 
> without patch:
> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
> 
> with patch:
> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED

Are you per chance running with a testsuite patch like this one?

--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
 				tmp, &size_tmp, &retval, NULL);
 	if (unpriv)
 		set_admin(false);
-	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
+	if (err && errno != EPERM) {
 		printf("Unexpected bpf_prog_test_run error ");
 		return err;
 	}

Without it, all the failures appear to be masked for me.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 13:21 ` Ilya Leoshkevich
@ 2019-08-27 13:46   ` Ilya Leoshkevich
  2019-08-27 14:21     ` Yauheni Kaliuta
  2019-08-27 14:25   ` Yauheni Kaliuta
  1 sibling, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-27 13:46 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 27.08.2019 um 15:21 schrieb Ilya Leoshkevich <iii@linux.ibm.com>:
> 
>> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
>> 
>> test_verifier (5.3-rc6):
>> 
>> without patch:
>> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
>> 
>> with patch:
>> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
> 
> Are you per chance running with a testsuite patch like this one?
> 
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
> 				tmp, &size_tmp, &retval, NULL);
> 	if (unpriv)
> 		set_admin(false);
> -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
> +	if (err && errno != EPERM) {
> 		printf("Unexpected bpf_prog_test_run error ");
> 		return err;
> 	}
> 
> Without it, all the failures appear to be masked for me.

Hmm, I'm sorry, I thought about it a bit more, and the patch I posted
above doesn't make any sense, because the failures you fixed are during
load, and not run time.

Now I think you are using CONFIG_BPF_JIT_ALWAYS_ON for your testing,
is that right? If yes, it would be nice to mention this in the commit
message.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 13:46   ` Ilya Leoshkevich
@ 2019-08-27 14:21     ` Yauheni Kaliuta
  2019-08-27 14:37       ` Ilya Leoshkevich
  0 siblings, 1 reply; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-27 14:21 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, daniel, jolsa

Hi, Ilya!

>>>>> On Tue, 27 Aug 2019 15:46:43 +0200, Ilya Leoshkevich  wrote:

 >> Am 27.08.2019 um 15:21 schrieb Ilya Leoshkevich <iii@linux.ibm.com>:
 >> 
 >>> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
 >>> 
 >>> test_verifier (5.3-rc6):
 >>> 
 >>> without patch:
 >>> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
 >>> 
 >>> with patch:
 >>> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
 >> 
 >> Are you per chance running with a testsuite patch like this one?
 >> 
 >> --- a/tools/testing/selftests/bpf/test_verifier.c
 >> +++ b/tools/testing/selftests/bpf/test_verifier.c
 >> @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
 >> tmp, &size_tmp, &retval, NULL);
 >> if (unpriv)
 >> set_admin(false);
 >> -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
 >> +	if (err && errno != EPERM) {
 >> printf("Unexpected bpf_prog_test_run error ");
 >> return err;
 >> }
 >> 
 >> Without it, all the failures appear to be masked for me.

 > Hmm, I'm sorry, I thought about it a bit more, and the patch I
 > posted above doesn't make any sense, because the failures you
 > fixed are during load, and not run time.

 > Now I think you are using CONFIG_BPF_JIT_ALWAYS_ON for your
 > testing, is that right? If yes, it would be nice to mention

Right.

 > this in the commit message.

Sure. Should I post non-RFC v2 or wait for some more comments?

-- 
WBR,
Yauheni Kaliuta

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 13:21 ` Ilya Leoshkevich
  2019-08-27 13:46   ` Ilya Leoshkevich
@ 2019-08-27 14:25   ` Yauheni Kaliuta
  2019-08-27 14:46     ` Ilya Leoshkevich
  1 sibling, 1 reply; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-27 14:25 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, daniel, jolsa

Hi, Ilya!

>>>>> On Tue, 27 Aug 2019 15:21:30 +0200, Ilya Leoshkevich  wrote:

 >> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
 >> 
 >> test_verifier (5.3-rc6):
 >> 
 >> without patch:
 >> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
 >> 
 >> with patch:
 >> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED

 > Are you per chance running with a testsuite patch like this one?

 > --- a/tools/testing/selftests/bpf/test_verifier.c
 > +++ b/tools/testing/selftests/bpf/test_verifier.c
 > @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
 >  				tmp, &size_tmp, &retval, NULL);
 >  	if (unpriv)
 >  		set_admin(false);
 > -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
 > +	if (err && errno != EPERM) {
 >  		printf("Unexpected bpf_prog_test_run error ");
 >  		return err;
 >  	}

 > Without it, all the failures appear to be masked for me.

BTW, I have several failures because of low BPF_SIZE_MAX. If I
increase it, some tests pass (#585/p ld_abs: vlan + abs, test 1),
but some crash (#587/p ld_abs: jump around ld_abs, haven't
found the reason yet).

Have you observed anything like that?

-- 
WBR,
Yauheni Kaliuta

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 14:21     ` Yauheni Kaliuta
@ 2019-08-27 14:37       ` Ilya Leoshkevich
  2019-08-27 14:48         ` Yauheni Kaliuta
  0 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-27 14:37 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 27.08.2019 um 16:21 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
> 
> Hi, Ilya!
> 
>>>>>> On Tue, 27 Aug 2019 15:46:43 +0200, Ilya Leoshkevich  wrote:
> 
>>> Am 27.08.2019 um 15:21 schrieb Ilya Leoshkevich <iii@linux.ibm.com>:
>>> 
>>>> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
>>>> 
>>>> test_verifier (5.3-rc6):
>>>> 
>>>> without patch:
>>>> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
>>>> 
>>>> with patch:
>>>> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
>>> 
>>> Are you per chance running with a testsuite patch like this one?
>>> 
>>> --- a/tools/testing/selftests/bpf/test_verifier.c
>>> +++ b/tools/testing/selftests/bpf/test_verifier.c
>>> @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
>>> tmp, &size_tmp, &retval, NULL);
>>> if (unpriv)
>>> set_admin(false);
>>> -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
>>> +	if (err && errno != EPERM) {
>>> printf("Unexpected bpf_prog_test_run error ");
>>> return err;
>>> }
>>> 
>>> Without it, all the failures appear to be masked for me.
> 
>> Hmm, I'm sorry, I thought about it a bit more, and the patch I
>> posted above doesn't make any sense, because the failures you
>> fixed are during load, and not run time.
> 
>> Now I think you are using CONFIG_BPF_JIT_ALWAYS_ON for your
>> testing, is that right? If yes, it would be nice to mention
> 
> Right.
> 
>> this in the commit message.
> 
> Sure. Should I post non-RFC v2 or wait for some more comments?

So far I only spotted a minor issue:

+		if (ret < 0)
+			return ret;

Right now bpf_jit_insn returns 0 or -1, but bpf_jit_get_func_addr
returns 0 or -errno. This does not affect anything in the end, but just
to be uniform, maybe return -1 here or -EINVAL in the default: branch?


I don't see any other obvious problems with the patch, but I'd like to
take some time to understand how exactly some parts of it work before
acking it. So I think it's fine to post a non-RFC version.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 14:25   ` Yauheni Kaliuta
@ 2019-08-27 14:46     ` Ilya Leoshkevich
  2019-08-27 15:05       ` Yauheni Kaliuta
  0 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-27 14:46 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 27.08.2019 um 16:25 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
> 
> Hi, Ilya!
> 
>>>>>> On Tue, 27 Aug 2019 15:21:30 +0200, Ilya Leoshkevich  wrote:
> 
>>> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
>>> 
>>> test_verifier (5.3-rc6):
>>> 
>>> without patch:
>>> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
>>> 
>>> with patch:
>>> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
> 
>> Are you per chance running with a testsuite patch like this one?
> 
>> --- a/tools/testing/selftests/bpf/test_verifier.c
>> +++ b/tools/testing/selftests/bpf/test_verifier.c
>> @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
>> 				tmp, &size_tmp, &retval, NULL);
>> 	if (unpriv)
>> 		set_admin(false);
>> -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
>> +	if (err && errno != EPERM) {
>> 		printf("Unexpected bpf_prog_test_run error ");
>> 		return err;
>> 	}
> 
>> Without it, all the failures appear to be masked for me.
> 
> BTW, I have several failures because of low BPF_SIZE_MAX. If I
> increase it, some tests pass (#585/p ld_abs: vlan + abs, test 1),
> but some crash (#587/p ld_abs: jump around ld_abs, haven't
> found the reason yet).
> 
> Have you observed anything like that?

Yes, this is because right now JIT generates clrj and friends,
which can jump only by +-32k. Improving this is actually my next task
(after fixing more or less "obvious" test suite problems).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 14:37       ` Ilya Leoshkevich
@ 2019-08-27 14:48         ` Yauheni Kaliuta
  0 siblings, 0 replies; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-27 14:48 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, daniel, jolsa

Hi, Ilya!

>>>>> On Tue, 27 Aug 2019 16:37:04 +0200, Ilya Leoshkevich  wrote:

 >> Am 27.08.2019 um 16:21 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
 >> 
 >> Hi, Ilya!
 >> 
 >>>>>>> On Tue, 27 Aug 2019 15:46:43 +0200, Ilya Leoshkevich  wrote:
 >> 
 >>>> Am 27.08.2019 um 15:21 schrieb Ilya Leoshkevich <iii@linux.ibm.com>:
 >>>> 
 >>>>> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
 >>>>> 
 >>>>> test_verifier (5.3-rc6):
 >>>>> 
 >>>>> without patch:
 >>>>> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
 >>>>> 
 >>>>> with patch:
 >>>>> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
 >>>> 
 >>>> Are you per chance running with a testsuite patch like this one?
 >>>> 
 >>>> --- a/tools/testing/selftests/bpf/test_verifier.c
 >>>> +++ b/tools/testing/selftests/bpf/test_verifier.c
 >>>> @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool
 >>>> unpriv, uint32_t expected_val,
 >>>> tmp, &size_tmp, &retval, NULL);
 >>>> if (unpriv)
 >>>> set_admin(false);
 >>>> -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
 >>>> +	if (err && errno != EPERM) {
 >>>> printf("Unexpected bpf_prog_test_run error ");
 >>>> return err;
 >>>> }
 >>>> 
 >>>> Without it, all the failures appear to be masked for me.
 >> 
 >>> Hmm, I'm sorry, I thought about it a bit more, and the patch I
 >>> posted above doesn't make any sense, because the failures you
 >>> fixed are during load, and not run time.
 >> 
 >>> Now I think you are using CONFIG_BPF_JIT_ALWAYS_ON for your
 >>> testing, is that right? If yes, it would be nice to mention
 >> 
 >> Right.
 >> 
 >>> this in the commit message.
 >> 
 >> Sure. Should I post non-RFC v2 or wait for some more comments?

 > So far I only spotted a minor issue:

 > +		if (ret < 0)
 > +			return ret;

 > Right now bpf_jit_insn returns 0 or -1, but
 > bpf_jit_get_func_addr returns 0 or -errno. This does not
 > affect anything in the end, but just to be uniform, maybe
 > return -1 here or -EINVAL in the default: branch?

ok. I choose "return -1" since changing default to -EINVAL sounds
as unrelated change to the patch.

 > I don't see any other obvious problems with the patch, but I'd
 > like to take some time to understand how exactly some parts of
 > it work before acking it. So I think it's fine to post a
 > non-RFC version.

Good, thanks!

-- 
WBR,
Yauheni Kaliuta

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] bpf: s390: add JIT support for multi-function programs
  2019-08-26 18:20 [RFC PATCH] bpf: s390: add JIT support for multi-function programs Yauheni Kaliuta
  2019-08-27 13:21 ` Ilya Leoshkevich
@ 2019-08-27 14:53 ` Yauheni Kaliuta
  2019-08-27 16:39   ` Ilya Leoshkevich
  2019-08-28 18:28 ` [PATCH v3] " Yauheni Kaliuta
  2 siblings, 1 reply; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-27 14:53 UTC (permalink / raw)
  To: bpf; +Cc: daniel, iii, jolsa

This adds support for bpf-to-bpf function calls in the s390 JIT
compiler. The JIT compiler converts the bpf call instructions to
native branch instructions. After a round of the usual passes, the
start addresses of the JITed images for the callee functions are
known. Finally, to fixup the branch target addresses, we need to
perform an extra pass.

Because of the address range in which JITed images are allocated on
s390, the offsets of the start addresses of these images from
__bpf_call_base are as large as 64 bits. So, for a function call,
the imm field of the instruction cannot be used to determine the
callee's address. Use bpf_jit_get_func_addr() helper instead.

The patch borrows a lot from:

8c11ea5ce13d bpf, arm64: fix getting subprog addr from aux for calls
e2c95a61656d bpf, ppc64: generalize fetching subprog into bpf_jit_get_func_addr
8484ce8306f9 bpf: powerpc64: add JIT support for multi-function programs

(including the commit message).

test_verifier (5.3-rc6 with CONFIG_BPF_JIT_ALWAYS_ON=y):

without patch:
Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED

with patch:
Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---

V1->V2:

- mention CONFIG_BPF_JIT_ALWAYS_ON=y in the commit message;
- in case of bpf_jit_get_func_addr() error return -1 to be consistent
  with the bpf_jit_insn() default branch return value.

---
 arch/s390/net/bpf_jit_comp.c | 63 +++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 11 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index e636728ab452..55fce1d7dca1 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -502,7 +502,8 @@ static void bpf_jit_epilogue(struct bpf_jit *jit, u32 stack_depth)
  * NOTE: Use noinline because for gcov (-fprofile-arcs) gcc allocates a lot of
  * stack space for the large switch statement.
  */
-static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i)
+static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
+				 int i, bool extra_pass)
 {
 	struct bpf_insn *insn = &fp->insnsi[i];
 	int jmp_off, last, insn_count = 1;
@@ -1011,10 +1012,14 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 	 */
 	case BPF_JMP | BPF_CALL:
 	{
-		/*
-		 * b0 = (__bpf_call_base + imm)(b1, b2, b3, b4, b5)
-		 */
-		const u64 func = (u64)__bpf_call_base + imm;
+		u64 func;
+		bool func_addr_fixed;
+		int ret;
+
+		ret = bpf_jit_get_func_addr(fp, insn, extra_pass,
+					    &func, &func_addr_fixed);
+		if (ret < 0)
+			return -1;
 
 		REG_SET_SEEN(BPF_REG_5);
 		jit->seen |= SEEN_FUNC;
@@ -1281,7 +1286,7 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 /*
  * Compile eBPF program into s390x code
  */
-static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
+static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp, bool extra_pass)
 {
 	int i, insn_count;
 
@@ -1290,7 +1295,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
 
 	bpf_jit_prologue(jit, fp->aux->stack_depth);
 	for (i = 0; i < fp->len; i += insn_count) {
-		insn_count = bpf_jit_insn(jit, fp, i);
+		insn_count = bpf_jit_insn(jit, fp, i, extra_pass);
 		if (insn_count < 0)
 			return -1;
 		/* Next instruction address */
@@ -1309,6 +1314,12 @@ bool bpf_jit_needs_zext(void)
 	return true;
 }
 
+
+struct s390_jit_data {
+	struct bpf_binary_header *header;
+	struct bpf_jit ctx;
+};
+
 /*
  * Compile eBPF program "fp"
  */
@@ -1316,7 +1327,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 {
 	struct bpf_prog *tmp, *orig_fp = fp;
 	struct bpf_binary_header *header;
+	struct s390_jit_data *jit_data;
 	bool tmp_blinded = false;
+	bool extra_pass = false;
 	struct bpf_jit jit;
 	int pass;
 
@@ -1335,6 +1348,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = tmp;
 	}
 
+	jit_data = fp->aux->jit_data;
+	if (!jit_data) {
+		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
+		if (!jit_data) {
+			fp = orig_fp;
+			goto out;
+		}
+		fp->aux->jit_data = jit_data;
+	}
+	if (jit_data->ctx.addrs) {
+		jit = jit_data->ctx;
+		header = jit_data->header;
+		extra_pass = true;
+		goto skip_init_ctx;
+	}
+
 	memset(&jit, 0, sizeof(jit));
 	jit.addrs = kcalloc(fp->len + 1, sizeof(*jit.addrs), GFP_KERNEL);
 	if (jit.addrs == NULL) {
@@ -1347,7 +1376,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	 *   - 3:   Calculate program size and addrs arrray
 	 */
 	for (pass = 1; pass <= 3; pass++) {
-		if (bpf_jit_prog(&jit, fp)) {
+		if (bpf_jit_prog(&jit, fp, extra_pass)) {
 			fp = orig_fp;
 			goto free_addrs;
 		}
@@ -1359,12 +1388,14 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = orig_fp;
 		goto free_addrs;
 	}
+
 	header = bpf_jit_binary_alloc(jit.size, &jit.prg_buf, 2, jit_fill_hole);
 	if (!header) {
 		fp = orig_fp;
 		goto free_addrs;
 	}
-	if (bpf_jit_prog(&jit, fp)) {
+skip_init_ctx:
+	if (bpf_jit_prog(&jit, fp, extra_pass)) {
 		bpf_jit_binary_free(header);
 		fp = orig_fp;
 		goto free_addrs;
@@ -1373,12 +1404,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
 		print_fn_code(jit.prg_buf, jit.size_prg);
 	}
-	bpf_jit_binary_lock_ro(header);
+	if (!fp->is_func || extra_pass) {
+		bpf_jit_binary_lock_ro(header);
+	} else {
+		jit_data->header = header;
+		jit_data->ctx = jit;
+	}
 	fp->bpf_func = (void *) jit.prg_buf;
 	fp->jited = 1;
 	fp->jited_len = jit.size;
+
+	if (!fp->is_func || extra_pass) {
 free_addrs:
-	kfree(jit.addrs);
+		kfree(jit.addrs);
+		kfree(jit_data);
+		fp->aux->jit_data = NULL;
+	}
 out:
 	if (tmp_blinded)
 		bpf_jit_prog_release_other(fp, fp == orig_fp ?
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] bpf: s390: add JIT support for multi-function programs
  2019-08-27 14:46     ` Ilya Leoshkevich
@ 2019-08-27 15:05       ` Yauheni Kaliuta
  0 siblings, 0 replies; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-27 15:05 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, daniel, jolsa

Hi, Ilya!

>>>>> On Tue, 27 Aug 2019 16:46:46 +0200, Ilya Leoshkevich  wrote:

 >> Am 27.08.2019 um 16:25 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
 >> 
 >> Hi, Ilya!
 >> 
 >>>>>>> On Tue, 27 Aug 2019 15:21:30 +0200, Ilya Leoshkevich  wrote:
 >> 
 >>>> Am 26.08.2019 um 20:20 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
 >>>> 
 >>>> test_verifier (5.3-rc6):
 >>>> 
 >>>> without patch:
 >>>> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
 >>>> 
 >>>> with patch:
 >>>> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
 >> 
 >>> Are you per chance running with a testsuite patch like this one?
 >> 
 >>> --- a/tools/testing/selftests/bpf/test_verifier.c
 >>> +++ b/tools/testing/selftests/bpf/test_verifier.c
 >>> @@ -846,7 +846,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
 >>> tmp, &size_tmp, &retval, NULL);
 >>> if (unpriv)
 >>> set_admin(false);
 >>> -	if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
 >>> +	if (err && errno != EPERM) {
 >>> printf("Unexpected bpf_prog_test_run error ");
 >>> return err;
 >>> }
 >> 
 >>> Without it, all the failures appear to be masked for me.
 >> 
 >> BTW, I have several failures because of low BPF_SIZE_MAX. If I
 >> increase it, some tests pass (#585/p ld_abs: vlan + abs, test 1),
 >> but some crash (#587/p ld_abs: jump around ld_abs, haven't
 >> found the reason yet).
 >> 
 >> Have you observed anything like that?

 > Yes, this is because right now JIT generates clrj and friends,
 > which can jump only by +-32k. Improving this is actually my
 > next task (after fixing more or less "obvious" test suite
 > problems).

ah, great. Sorry for the noise.

-- 
WBR,
Yauheni Kaliuta

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] bpf: s390: add JIT support for multi-function programs
  2019-08-27 14:53 ` [PATCH v2] " Yauheni Kaliuta
@ 2019-08-27 16:39   ` Ilya Leoshkevich
  2019-08-28  9:11     ` Ilya Leoshkevich
  0 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-27 16:39 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 27.08.2019 um 16:53 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
> 
> @@ -1316,7 +1327,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> {
> 	struct bpf_prog *tmp, *orig_fp = fp;
> 	struct bpf_binary_header *header;
> +	struct s390_jit_data *jit_data;
> 	bool tmp_blinded = false;
> +	bool extra_pass = false;
> 	struct bpf_jit jit;
> 	int pass;
> 
> @@ -1335,6 +1348,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> 		fp = tmp;
> 	}
> 
> +	jit_data = fp->aux->jit_data;
> +	if (!jit_data) {
> +		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
> +		if (!jit_data) {
> +			fp = orig_fp;
> +			goto out;
> +		}
> +		fp->aux->jit_data = jit_data;
> +	}
> +	if (jit_data->ctx.addrs) {
> +		jit = jit_data->ctx;
> +		header = jit_data->header;
> +		extra_pass = true;
> +		goto skip_init_ctx;
> +	}
> +

I've noticed that I'm getting the following warning, presumably because
of the added goto skip_init_ctx:

linux/arch/s390/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':
linux/arch/s390/net/bpf_jit_comp.c:1406:3: warning: 'pass' may be used uninitialized in this function [-Wmaybe-uninitialized]
   bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);

Maybe set the initial value of pass to 1?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] bpf: s390: add JIT support for multi-function programs
  2019-08-27 16:39   ` Ilya Leoshkevich
@ 2019-08-28  9:11     ` Ilya Leoshkevich
  0 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-28  9:11 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 27.08.2019 um 18:39 schrieb Ilya Leoshkevich <iii@linux.ibm.com>:
> 
>> Am 27.08.2019 um 16:53 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
>> 
>> @@ -1316,7 +1327,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>> {
>> 	struct bpf_prog *tmp, *orig_fp = fp;
>> 	struct bpf_binary_header *header;
>> +	struct s390_jit_data *jit_data;
>> 	bool tmp_blinded = false;
>> +	bool extra_pass = false;
>> 	struct bpf_jit jit;
>> 	int pass;
>> 
>> @@ -1335,6 +1348,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>> 		fp = tmp;
>> 	}
>> 
>> +	jit_data = fp->aux->jit_data;
>> +	if (!jit_data) {
>> +		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
>> +		if (!jit_data) {
>> +			fp = orig_fp;
>> +			goto out;
>> +		}
>> +		fp->aux->jit_data = jit_data;
>> +	}
>> +	if (jit_data->ctx.addrs) {
>> +		jit = jit_data->ctx;
>> +		header = jit_data->header;
>> +		extra_pass = true;
>> +		goto skip_init_ctx;
>> +	}
>> +
> 
> I've noticed that I'm getting the following warning, presumably because
> of the added goto skip_init_ctx:
> 
> linux/arch/s390/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':
> linux/arch/s390/net/bpf_jit_comp.c:1406:3: warning: 'pass' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
> 
> Maybe set the initial value of pass to 1?

... or save pass in s390_jit_data and do the following?

	pass = jit_data->pass + 1;
	goto skip_init_ctx;

I've also noticed that

	$ git show --format=email | scripts/checkpatch.pl --strict

complains - could you please fix these warnings?


I've finally reviewed the actual logic of the patch, and it looks good
to me.  The integration with the common code is somewhat involved, but
it follows the established architecture.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] bpf: s390: add JIT support for multi-function programs
  2019-08-26 18:20 [RFC PATCH] bpf: s390: add JIT support for multi-function programs Yauheni Kaliuta
  2019-08-27 13:21 ` Ilya Leoshkevich
  2019-08-27 14:53 ` [PATCH v2] " Yauheni Kaliuta
@ 2019-08-28 18:28 ` Yauheni Kaliuta
  2019-08-29  8:52   ` Ilya Leoshkevich
  2019-08-30 23:22   ` Daniel Borkmann
  2 siblings, 2 replies; 15+ messages in thread
From: Yauheni Kaliuta @ 2019-08-28 18:28 UTC (permalink / raw)
  To: bpf; +Cc: daniel, iii, jolsa

This adds support for bpf-to-bpf function calls in the s390 JIT
compiler. The JIT compiler converts the bpf call instructions to
native branch instructions. After a round of the usual passes, the
start addresses of the JITed images for the callee functions are
known. Finally, to fixup the branch target addresses, we need to
perform an extra pass.

Because of the address range in which JITed images are allocated on
s390, the offsets of the start addresses of these images from
__bpf_call_base are as large as 64 bits. So, for a function call,
the imm field of the instruction cannot be used to determine the
callee's address. Use bpf_jit_get_func_addr() helper instead.

The patch borrows a lot from:

commit 8c11ea5ce13d ("bpf, arm64: fix getting subprog addr from aux
for calls")

commit e2c95a61656d ("bpf, ppc64: generalize fetching subprog into
bpf_jit_get_func_addr")

commit 8484ce8306f9 ("bpf: powerpc64: add JIT support for
multi-function programs")

(including the commit message).

test_verifier (5.3-rc6 with CONFIG_BPF_JIT_ALWAYS_ON=y):

without patch:
Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED

with patch:
Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 arch/s390/net/bpf_jit_comp.c | 66 ++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 955eb355c2fd..b6801d854c77 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -502,7 +502,8 @@ static void bpf_jit_epilogue(struct bpf_jit *jit, u32 stack_depth)
  * NOTE: Use noinline because for gcov (-fprofile-arcs) gcc allocates a lot of
  * stack space for the large switch statement.
  */
-static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i)
+static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
+				 int i, bool extra_pass)
 {
 	struct bpf_insn *insn = &fp->insnsi[i];
 	int jmp_off, last, insn_count = 1;
@@ -1011,10 +1012,14 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 	 */
 	case BPF_JMP | BPF_CALL:
 	{
-		/*
-		 * b0 = (__bpf_call_base + imm)(b1, b2, b3, b4, b5)
-		 */
-		const u64 func = (u64)__bpf_call_base + imm;
+		u64 func;
+		bool func_addr_fixed;
+		int ret;
+
+		ret = bpf_jit_get_func_addr(fp, insn, extra_pass,
+					    &func, &func_addr_fixed);
+		if (ret < 0)
+			return -1;
 
 		REG_SET_SEEN(BPF_REG_5);
 		jit->seen |= SEEN_FUNC;
@@ -1283,7 +1288,8 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 /*
  * Compile eBPF program into s390x code
  */
-static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
+static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp,
+			bool extra_pass)
 {
 	int i, insn_count;
 
@@ -1292,7 +1298,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
 
 	bpf_jit_prologue(jit, fp->aux->stack_depth);
 	for (i = 0; i < fp->len; i += insn_count) {
-		insn_count = bpf_jit_insn(jit, fp, i);
+		insn_count = bpf_jit_insn(jit, fp, i, extra_pass);
 		if (insn_count < 0)
 			return -1;
 		/* Next instruction address */
@@ -1311,6 +1317,12 @@ bool bpf_jit_needs_zext(void)
 	return true;
 }
 
+struct s390_jit_data {
+	struct bpf_binary_header *header;
+	struct bpf_jit ctx;
+	int pass;
+};
+
 /*
  * Compile eBPF program "fp"
  */
@@ -1318,7 +1330,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 {
 	struct bpf_prog *tmp, *orig_fp = fp;
 	struct bpf_binary_header *header;
+	struct s390_jit_data *jit_data;
 	bool tmp_blinded = false;
+	bool extra_pass = false;
 	struct bpf_jit jit;
 	int pass;
 
@@ -1337,6 +1351,23 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = tmp;
 	}
 
+	jit_data = fp->aux->jit_data;
+	if (!jit_data) {
+		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
+		if (!jit_data) {
+			fp = orig_fp;
+			goto out;
+		}
+		fp->aux->jit_data = jit_data;
+	}
+	if (jit_data->ctx.addrs) {
+		jit = jit_data->ctx;
+		header = jit_data->header;
+		extra_pass = true;
+		pass = jit_data->pass + 1;
+		goto skip_init_ctx;
+	}
+
 	memset(&jit, 0, sizeof(jit));
 	jit.addrs = kcalloc(fp->len + 1, sizeof(*jit.addrs), GFP_KERNEL);
 	if (jit.addrs == NULL) {
@@ -1349,7 +1380,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	 *   - 3:   Calculate program size and addrs arrray
 	 */
 	for (pass = 1; pass <= 3; pass++) {
-		if (bpf_jit_prog(&jit, fp)) {
+		if (bpf_jit_prog(&jit, fp, extra_pass)) {
 			fp = orig_fp;
 			goto free_addrs;
 		}
@@ -1361,12 +1392,14 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		fp = orig_fp;
 		goto free_addrs;
 	}
+
 	header = bpf_jit_binary_alloc(jit.size, &jit.prg_buf, 2, jit_fill_hole);
 	if (!header) {
 		fp = orig_fp;
 		goto free_addrs;
 	}
-	if (bpf_jit_prog(&jit, fp)) {
+skip_init_ctx:
+	if (bpf_jit_prog(&jit, fp, extra_pass)) {
 		bpf_jit_binary_free(header);
 		fp = orig_fp;
 		goto free_addrs;
@@ -1375,12 +1408,23 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 		bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
 		print_fn_code(jit.prg_buf, jit.size_prg);
 	}
-	bpf_jit_binary_lock_ro(header);
+	if (!fp->is_func || extra_pass) {
+		bpf_jit_binary_lock_ro(header);
+	} else {
+		jit_data->header = header;
+		jit_data->ctx = jit;
+		jit_data->pass = pass;
+	}
 	fp->bpf_func = (void *) jit.prg_buf;
 	fp->jited = 1;
 	fp->jited_len = jit.size;
+
+	if (!fp->is_func || extra_pass) {
 free_addrs:
-	kfree(jit.addrs);
+		kfree(jit.addrs);
+		kfree(jit_data);
+		fp->aux->jit_data = NULL;
+	}
 out:
 	if (tmp_blinded)
 		bpf_jit_prog_release_other(fp, fp == orig_fp ?
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] bpf: s390: add JIT support for multi-function programs
  2019-08-28 18:28 ` [PATCH v3] " Yauheni Kaliuta
@ 2019-08-29  8:52   ` Ilya Leoshkevich
  2019-08-30 23:22   ` Daniel Borkmann
  1 sibling, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2019-08-29  8:52 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: bpf, daniel, jolsa

> Am 28.08.2019 um 20:28 schrieb Yauheni Kaliuta <yauheni.kaliuta@redhat.com>:
> 
> This adds support for bpf-to-bpf function calls in the s390 JIT
> compiler. The JIT compiler converts the bpf call instructions to
> native branch instructions. After a round of the usual passes, the
> start addresses of the JITed images for the callee functions are
> known. Finally, to fixup the branch target addresses, we need to
> perform an extra pass.
> 
> Because of the address range in which JITed images are allocated on
> s390, the offsets of the start addresses of these images from
> __bpf_call_base are as large as 64 bits. So, for a function call,
> the imm field of the instruction cannot be used to determine the
> callee's address. Use bpf_jit_get_func_addr() helper instead.
> 
> The patch borrows a lot from:
> 
> commit 8c11ea5ce13d ("bpf, arm64: fix getting subprog addr from aux
> for calls")
> 
> commit e2c95a61656d ("bpf, ppc64: generalize fetching subprog into
> bpf_jit_get_func_addr")
> 
> commit 8484ce8306f9 ("bpf: powerpc64: add JIT support for
> multi-function programs")
> 
> (including the commit message).
> 
> test_verifier (5.3-rc6 with CONFIG_BPF_JIT_ALWAYS_ON=y):
> 
> without patch:
> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
> 
> with patch:
> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
> 
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
> arch/s390/net/bpf_jit_comp.c | 66 ++++++++++++++++++++++++++++++------
> 1 file changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index 955eb355c2fd..b6801d854c77 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -502,7 +502,8 @@ static void bpf_jit_epilogue(struct bpf_jit *jit, u32 stack_depth)
>  * NOTE: Use noinline because for gcov (-fprofile-arcs) gcc allocates a lot of
>  * stack space for the large switch statement.
>  */
> -static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i)
> +static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
> +				 int i, bool extra_pass)
> {
> 	struct bpf_insn *insn = &fp->insnsi[i];
> 	int jmp_off, last, insn_count = 1;
> @@ -1011,10 +1012,14 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
> 	 */
> 	case BPF_JMP | BPF_CALL:
> 	{
> -		/*
> -		 * b0 = (__bpf_call_base + imm)(b1, b2, b3, b4, b5)
> -		 */
> -		const u64 func = (u64)__bpf_call_base + imm;
> +		u64 func;
> +		bool func_addr_fixed;
> +		int ret;
> +
> +		ret = bpf_jit_get_func_addr(fp, insn, extra_pass,
> +					    &func, &func_addr_fixed);
> +		if (ret < 0)
> +			return -1;
> 
> 		REG_SET_SEEN(BPF_REG_5);
> 		jit->seen |= SEEN_FUNC;
> @@ -1283,7 +1288,8 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
> /*
>  * Compile eBPF program into s390x code
>  */
> -static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
> +static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp,
> +			bool extra_pass)
> {
> 	int i, insn_count;
> 
> @@ -1292,7 +1298,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
> 
> 	bpf_jit_prologue(jit, fp->aux->stack_depth);
> 	for (i = 0; i < fp->len; i += insn_count) {
> -		insn_count = bpf_jit_insn(jit, fp, i);
> +		insn_count = bpf_jit_insn(jit, fp, i, extra_pass);
> 		if (insn_count < 0)
> 			return -1;
> 		/* Next instruction address */
> @@ -1311,6 +1317,12 @@ bool bpf_jit_needs_zext(void)
> 	return true;
> }
> 
> +struct s390_jit_data {
> +	struct bpf_binary_header *header;
> +	struct bpf_jit ctx;
> +	int pass;
> +};
> +
> /*
>  * Compile eBPF program "fp"
>  */
> @@ -1318,7 +1330,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> {
> 	struct bpf_prog *tmp, *orig_fp = fp;
> 	struct bpf_binary_header *header;
> +	struct s390_jit_data *jit_data;
> 	bool tmp_blinded = false;
> +	bool extra_pass = false;
> 	struct bpf_jit jit;
> 	int pass;
> 
> @@ -1337,6 +1351,23 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> 		fp = tmp;
> 	}
> 
> +	jit_data = fp->aux->jit_data;
> +	if (!jit_data) {
> +		jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL);
> +		if (!jit_data) {
> +			fp = orig_fp;
> +			goto out;
> +		}
> +		fp->aux->jit_data = jit_data;
> +	}
> +	if (jit_data->ctx.addrs) {
> +		jit = jit_data->ctx;
> +		header = jit_data->header;
> +		extra_pass = true;
> +		pass = jit_data->pass + 1;
> +		goto skip_init_ctx;
> +	}
> +
> 	memset(&jit, 0, sizeof(jit));
> 	jit.addrs = kcalloc(fp->len + 1, sizeof(*jit.addrs), GFP_KERNEL);
> 	if (jit.addrs == NULL) {
> @@ -1349,7 +1380,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> 	 *   - 3:   Calculate program size and addrs arrray
> 	 */
> 	for (pass = 1; pass <= 3; pass++) {
> -		if (bpf_jit_prog(&jit, fp)) {
> +		if (bpf_jit_prog(&jit, fp, extra_pass)) {
> 			fp = orig_fp;
> 			goto free_addrs;
> 		}
> @@ -1361,12 +1392,14 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> 		fp = orig_fp;
> 		goto free_addrs;
> 	}
> +
> 	header = bpf_jit_binary_alloc(jit.size, &jit.prg_buf, 2, jit_fill_hole);
> 	if (!header) {
> 		fp = orig_fp;
> 		goto free_addrs;
> 	}
> -	if (bpf_jit_prog(&jit, fp)) {
> +skip_init_ctx:
> +	if (bpf_jit_prog(&jit, fp, extra_pass)) {
> 		bpf_jit_binary_free(header);
> 		fp = orig_fp;
> 		goto free_addrs;
> @@ -1375,12 +1408,23 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> 		bpf_jit_dump(fp->len, jit.size, pass, jit.prg_buf);
> 		print_fn_code(jit.prg_buf, jit.size_prg);
> 	}
> -	bpf_jit_binary_lock_ro(header);
> +	if (!fp->is_func || extra_pass) {
> +		bpf_jit_binary_lock_ro(header);
> +	} else {
> +		jit_data->header = header;
> +		jit_data->ctx = jit;
> +		jit_data->pass = pass;
> +	}
> 	fp->bpf_func = (void *) jit.prg_buf;
> 	fp->jited = 1;
> 	fp->jited_len = jit.size;
> +
> +	if (!fp->is_func || extra_pass) {
> free_addrs:
> -	kfree(jit.addrs);
> +		kfree(jit.addrs);
> +		kfree(jit_data);
> +		fp->aux->jit_data = NULL;
> +	}
> out:
> 	if (tmp_blinded)
> 		bpf_jit_prog_release_other(fp, fp == orig_fp ?
> -- 
> 2.22.0

Thank you!

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] bpf: s390: add JIT support for multi-function programs
  2019-08-28 18:28 ` [PATCH v3] " Yauheni Kaliuta
  2019-08-29  8:52   ` Ilya Leoshkevich
@ 2019-08-30 23:22   ` Daniel Borkmann
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel Borkmann @ 2019-08-30 23:22 UTC (permalink / raw)
  To: Yauheni Kaliuta, bpf; +Cc: iii, jolsa

On 8/28/19 8:28 PM, Yauheni Kaliuta wrote:
> This adds support for bpf-to-bpf function calls in the s390 JIT
> compiler. The JIT compiler converts the bpf call instructions to
> native branch instructions. After a round of the usual passes, the
> start addresses of the JITed images for the callee functions are
> known. Finally, to fixup the branch target addresses, we need to
> perform an extra pass.
> 
> Because of the address range in which JITed images are allocated on
> s390, the offsets of the start addresses of these images from
> __bpf_call_base are as large as 64 bits. So, for a function call,
> the imm field of the instruction cannot be used to determine the
> callee's address. Use bpf_jit_get_func_addr() helper instead.
> 
> The patch borrows a lot from:
> 
> commit 8c11ea5ce13d ("bpf, arm64: fix getting subprog addr from aux
> for calls")
> 
> commit e2c95a61656d ("bpf, ppc64: generalize fetching subprog into
> bpf_jit_get_func_addr")
> 
> commit 8484ce8306f9 ("bpf: powerpc64: add JIT support for
> multi-function programs")
> 
> (including the commit message).
> 
> test_verifier (5.3-rc6 with CONFIG_BPF_JIT_ALWAYS_ON=y):
> 
> without patch:
> Summary: 1501 PASSED, 0 SKIPPED, 47 FAILED
> 
> with patch:
> Summary: 1540 PASSED, 0 SKIPPED, 8 FAILED
> 
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>

Applied, thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-08-30 23:22 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26 18:20 [RFC PATCH] bpf: s390: add JIT support for multi-function programs Yauheni Kaliuta
2019-08-27 13:21 ` Ilya Leoshkevich
2019-08-27 13:46   ` Ilya Leoshkevich
2019-08-27 14:21     ` Yauheni Kaliuta
2019-08-27 14:37       ` Ilya Leoshkevich
2019-08-27 14:48         ` Yauheni Kaliuta
2019-08-27 14:25   ` Yauheni Kaliuta
2019-08-27 14:46     ` Ilya Leoshkevich
2019-08-27 15:05       ` Yauheni Kaliuta
2019-08-27 14:53 ` [PATCH v2] " Yauheni Kaliuta
2019-08-27 16:39   ` Ilya Leoshkevich
2019-08-28  9:11     ` Ilya Leoshkevich
2019-08-28 18:28 ` [PATCH v3] " Yauheni Kaliuta
2019-08-29  8:52   ` Ilya Leoshkevich
2019-08-30 23:22   ` Daniel Borkmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.