From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: pull-request: bpf-next 2017-12-18 Date: Fri, 22 Dec 2017 00:48:22 +0100 Message-ID: <9a25f2dd-596f-5aae-65d9-5730406848d0@iogearbox.net> References: <20171218.105153.395572657387757515.davem@davemloft.net> <20171219062828.r6xadsdenfn2v5i2@ast-mbp> <20171220.161644.274941609957500499.davem@davemloft.net> <20171221.112819.2190571738067203400.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: ast@kernel.org, netdev@vger.kernel.org To: David Miller , alexei.starovoitov@gmail.com Return-path: Received: from www62.your-server.de ([213.133.104.62]:35255 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752148AbdLUXsa (ORCPT ); Thu, 21 Dec 2017 18:48:30 -0500 In-Reply-To: <20171221.112819.2190571738067203400.davem@davemloft.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 12/21/2017 05:28 PM, David Miller wrote: > From: David Miller > Date: Wed, 20 Dec 2017 16:16:44 -0500 (EST) > >> I think I understand how this new stuff works, I'll take a stab at >> doing the sparc64 JIT bits. > > This patch should do it, please queue up for bpf-next. > > But this is really overkill on sparc64. > > No matter where you relocate the call destination to, the size of the > program and the code output will be identical except for the call > instruction PC relative offset field. > > So at some point as a follow-up I should change this code to simply > scan the insns for the function calls and fixup the offsets, rather > than do a full set of code generation passes. > > Thanks. > > ==================== > bpf: sparc64: Add JIT support for multi-function programs. > > Modelled strongly upon the arm64 implementation. > > Signed-off-by: David S. Miller > > diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c > index a2f1b5e..4ee417f 100644 > --- a/arch/sparc/net/bpf_jit_comp_64.c > +++ b/arch/sparc/net/bpf_jit_comp_64.c > @@ -1507,11 +1507,19 @@ static void jit_fill_hole(void *area, unsigned int size) > *ptr++ = 0x91d02005; /* ta 5 */ > } > > +struct sparc64_jit_data { > + struct bpf_binary_header *header; > + u8 *image; > + struct jit_ctx ctx; > +}; > + > struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) > { > struct bpf_prog *tmp, *orig_prog = prog; > + struct sparc64_jit_data *jit_data; > struct bpf_binary_header *header; > bool tmp_blinded = false; > + bool extra_pass = false; > struct jit_ctx ctx; > u32 image_size; > u8 *image_ptr; > @@ -1531,13 +1539,30 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) > prog = tmp; > } > > + jit_data = prog->aux->jit_data; > + if (!jit_data) { > + jit_data = kzalloc(sizeof(*jit_data), GFP_KERNEL); > + if (!jit_data) { > + prog = orig_prog; > + goto out; > + } Looks good, one thing: If I spot this correctly, isn't here a ... prog->aux->jit_data = jit_data; ... missing? Otherwise the context from the initial pass is neither saved for the extra pass nor freed. > + } > + if (jit_data->ctx.offset) { > + ctx = jit_data->ctx; > + image_ptr = jit_data->image; > + header = jit_data->header; > + extra_pass = true; > + image_size = sizeof(u32) * ctx.idx; > + goto skip_init_ctx; > + } > + > memset(&ctx, 0, sizeof(ctx)); > ctx.prog = prog; > > ctx.offset = kcalloc(prog->len, sizeof(unsigned int), GFP_KERNEL); > if (ctx.offset == NULL) { > prog = orig_prog; > - goto out; > + goto out_off; > } > > /* Fake pass to detect features used, and get an accurate assessment > @@ -1560,7 +1585,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) > } > > ctx.image = (u32 *)image_ptr; > - > +skip_init_ctx: > for (pass = 1; pass < 3; pass++) { > ctx.idx = 0; > > @@ -1591,14 +1616,24 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) > > bpf_flush_icache(header, (u8 *)header + (header->pages * PAGE_SIZE)); > > - bpf_jit_binary_lock_ro(header); > + if (!prog->is_func || extra_pass) { > + bpf_jit_binary_lock_ro(header); > + } else { > + jit_data->ctx = ctx; > + jit_data->image = image_ptr; > + jit_data->header = header; > + } > > prog->bpf_func = (void *)ctx.image; > prog->jited = 1; > prog->jited_len = image_size; > > + if (!prog->is_func || extra_pass) { > out_off: > - kfree(ctx.offset); > + kfree(ctx.offset); > + kfree(jit_data); > + prog->aux->jit_data = NULL; > + } > out: > if (tmp_blinded) > bpf_jit_prog_release_other(prog, prog == orig_prog ? >