From: Daniel Borkmann <daniel@iogearbox.net> To: bjorn.topel@gmail.com, linux-riscv@lists.infradead.org, ast@kernel.org, netdev@vger.kernel.org Cc: palmer@sifive.com, hch@infradead.org Subject: Re: [PATCH bpf-next 1/3] bpf, riscv: add BPF JIT for RV64G Date: Mon, 4 Feb 2019 21:06:40 +0100 [thread overview] Message-ID: <88cdae60-494e-6294-b2c1-10b9cbeb95ac@iogearbox.net> (raw) In-Reply-To: <20190203115132.8766-2-bjorn.topel@gmail.com> On 02/03/2019 12:51 PM, bjorn.topel@gmail.com wrote: > From: Björn Töpel <bjorn.topel@gmail.com> > > This commit adds BPF JIT for RV64G. > > The JIT is a two-pass JIT, and has a dynamic prolog/epilogue (similar > to the MIPS64 BPF JIT) instead of static ones (e.g. x86_64). > > At the moment the RISC-V Linux port does not support HAVE_KPROBES, > which means that CONFIG_BPF_EVENTS is not supported. Thus, no tests > involving BPF_PROG_TYPE_TRACEPOINT passes. > > Further, the implementation does not support "far branching" (>4KiB). > > The implementation passes all the test_bpf.ko tests: > test_bpf: Summary: 378 PASSED, 0 FAILED, [366/366 JIT'ed] > > All the tail_call tests in the selftest/bpf/test_verifier program > passes. > > All tests where done on QEMU (QEMU emulator version 3.1.50 > (v3.1.0-688-g8ae951fbc106)). > > Signed-off-by: Björn Töpel <bjorn.topel@gmail.com> Some minor comments: Looks like all the BPF_JMP32 instructions are missing. Would probably make sense to include these into the initial merge as well unless there is some good reason not to; presumably the test_verifier parts with BPF_JMP32 haven't been tried out? [...] > + > +enum { > + RV_CTX_F_SEEN_TAIL_CALL = 0, > + RV_CTX_F_SEEN_CALL = RV_REG_RA, > + RV_CTX_F_SEEN_S1 = RV_REG_S1, > + RV_CTX_F_SEEN_S2 = RV_REG_S2, > + RV_CTX_F_SEEN_S3 = RV_REG_S3, > + RV_CTX_F_SEEN_S4 = RV_REG_S4, > + RV_CTX_F_SEEN_S5 = RV_REG_S5, > + RV_CTX_F_SEEN_S6 = RV_REG_S6, > +}; > + > +struct rv_jit_context { > + struct bpf_prog *prog; > + u32 *insns; /* RV insns */ > + int ninsns; > + int epilogue_offset; > + int *offset; /* BPF to RV */ > + unsigned long flags; > + int stack_size; > +}; > + > +struct rv_jit_data { > + struct bpf_binary_header *header; > + u8 *image; > + struct rv_jit_context ctx; > +}; > + > +static u8 bpf_to_rv_reg(int bpf_reg, struct rv_jit_context *ctx) > +{ > + u8 reg = regmap[bpf_reg]; > + > + switch (reg) { > + case RV_CTX_F_SEEN_S1: > + case RV_CTX_F_SEEN_S2: > + case RV_CTX_F_SEEN_S3: > + case RV_CTX_F_SEEN_S4: > + case RV_CTX_F_SEEN_S5: > + case RV_CTX_F_SEEN_S6: > + __set_bit(reg, &ctx->flags); > + } > + return reg; > +}; > + > +static bool seen_reg(int reg, struct rv_jit_context *ctx) > +{ > + switch (reg) { > + case RV_CTX_F_SEEN_CALL: > + case RV_CTX_F_SEEN_S1: > + case RV_CTX_F_SEEN_S2: > + case RV_CTX_F_SEEN_S3: > + case RV_CTX_F_SEEN_S4: > + case RV_CTX_F_SEEN_S5: > + case RV_CTX_F_SEEN_S6: > + return test_bit(reg, &ctx->flags); > + } > + return false; > +} > + > +static void mark_call(struct rv_jit_context *ctx) > +{ > + __set_bit(RV_CTX_F_SEEN_CALL, &ctx->flags); > +} > + > +static bool seen_call(struct rv_jit_context *ctx) > +{ > + return seen_reg(RV_REG_RA, ctx); > +} Just nit: probably might be more obvious to remove this asymmetry in seen_reg() and do __set_bit()/test_bit() for RV_CTX_F_SEEN_CALL similar like below. > +static void mark_tail_call(struct rv_jit_context *ctx) > +{ > + __set_bit(RV_CTX_F_SEEN_TAIL_CALL, &ctx->flags); > +} > + > +static bool seen_tail_call(struct rv_jit_context *ctx) > +{ > + return test_bit(RV_CTX_F_SEEN_TAIL_CALL, &ctx->flags); > +} > + > +static u8 rv_tail_call_reg(struct rv_jit_context *ctx) > +{ > + mark_tail_call(ctx); > + > + if (seen_call(ctx)) { > + __set_bit(RV_CTX_F_SEEN_S6, &ctx->flags); > + return RV_REG_S6; > + } > + return RV_REG_A6; > +} > + > +static void emit(const u32 insn, struct rv_jit_context *ctx) > +{ > + if (ctx->insns) > + ctx->insns[ctx->ninsns] = insn; > + > + ctx->ninsns++; > +} > + > +static u32 rv_r_insn(u8 funct7, u8 rs2, u8 rs1, u8 funct3, u8 rd, u8 opcode) > +{ [...] > + /* Allocate image, now that we know the size. */ > + image_size = sizeof(u32) * ctx->ninsns; > + jit_data->header = bpf_jit_binary_alloc(image_size, &jit_data->image, > + sizeof(u32), > + bpf_fill_ill_insns); > + if (!jit_data->header) { > + prog = orig_prog; > + goto out_offset; > + } > + > + /* Second, real pass, that acutally emits the image. */ > + ctx->insns = (u32 *)jit_data->image; > +skip_init_ctx: > + ctx->ninsns = 0; > + > + build_prologue(ctx); > + if (build_body(ctx, extra_pass)) { > + bpf_jit_binary_free(jit_data->header); > + prog = orig_prog; > + goto out_offset; > + } > + build_epilogue(ctx); > + > + if (bpf_jit_enable > 1) > + bpf_jit_dump(prog->len, image_size, 2, ctx->insns); > + > + prog->bpf_func = (void *)ctx->insns; > + prog->jited = 1; > + prog->jited_len = image_size; > + > + bpf_flush_icache(jit_data->header, (u8 *)ctx->insns + ctx->ninsns); Shouldn't this be '(u32 *)ctx->insns + ctx->ninsns' to cover the range? > + > + if (!prog->is_func || extra_pass) { > +out_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 ? > + tmp : orig_prog); > + return prog; > +} >
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Borkmann <daniel@iogearbox.net> To: bjorn.topel@gmail.com, linux-riscv@lists.infradead.org, ast@kernel.org, netdev@vger.kernel.org Cc: hch@infradead.org, palmer@sifive.com Subject: Re: [PATCH bpf-next 1/3] bpf, riscv: add BPF JIT for RV64G Date: Mon, 4 Feb 2019 21:06:40 +0100 [thread overview] Message-ID: <88cdae60-494e-6294-b2c1-10b9cbeb95ac@iogearbox.net> (raw) In-Reply-To: <20190203115132.8766-2-bjorn.topel@gmail.com> On 02/03/2019 12:51 PM, bjorn.topel@gmail.com wrote: > From: Björn Töpel <bjorn.topel@gmail.com> > > This commit adds BPF JIT for RV64G. > > The JIT is a two-pass JIT, and has a dynamic prolog/epilogue (similar > to the MIPS64 BPF JIT) instead of static ones (e.g. x86_64). > > At the moment the RISC-V Linux port does not support HAVE_KPROBES, > which means that CONFIG_BPF_EVENTS is not supported. Thus, no tests > involving BPF_PROG_TYPE_TRACEPOINT passes. > > Further, the implementation does not support "far branching" (>4KiB). > > The implementation passes all the test_bpf.ko tests: > test_bpf: Summary: 378 PASSED, 0 FAILED, [366/366 JIT'ed] > > All the tail_call tests in the selftest/bpf/test_verifier program > passes. > > All tests where done on QEMU (QEMU emulator version 3.1.50 > (v3.1.0-688-g8ae951fbc106)). > > Signed-off-by: Björn Töpel <bjorn.topel@gmail.com> Some minor comments: Looks like all the BPF_JMP32 instructions are missing. Would probably make sense to include these into the initial merge as well unless there is some good reason not to; presumably the test_verifier parts with BPF_JMP32 haven't been tried out? [...] > + > +enum { > + RV_CTX_F_SEEN_TAIL_CALL = 0, > + RV_CTX_F_SEEN_CALL = RV_REG_RA, > + RV_CTX_F_SEEN_S1 = RV_REG_S1, > + RV_CTX_F_SEEN_S2 = RV_REG_S2, > + RV_CTX_F_SEEN_S3 = RV_REG_S3, > + RV_CTX_F_SEEN_S4 = RV_REG_S4, > + RV_CTX_F_SEEN_S5 = RV_REG_S5, > + RV_CTX_F_SEEN_S6 = RV_REG_S6, > +}; > + > +struct rv_jit_context { > + struct bpf_prog *prog; > + u32 *insns; /* RV insns */ > + int ninsns; > + int epilogue_offset; > + int *offset; /* BPF to RV */ > + unsigned long flags; > + int stack_size; > +}; > + > +struct rv_jit_data { > + struct bpf_binary_header *header; > + u8 *image; > + struct rv_jit_context ctx; > +}; > + > +static u8 bpf_to_rv_reg(int bpf_reg, struct rv_jit_context *ctx) > +{ > + u8 reg = regmap[bpf_reg]; > + > + switch (reg) { > + case RV_CTX_F_SEEN_S1: > + case RV_CTX_F_SEEN_S2: > + case RV_CTX_F_SEEN_S3: > + case RV_CTX_F_SEEN_S4: > + case RV_CTX_F_SEEN_S5: > + case RV_CTX_F_SEEN_S6: > + __set_bit(reg, &ctx->flags); > + } > + return reg; > +}; > + > +static bool seen_reg(int reg, struct rv_jit_context *ctx) > +{ > + switch (reg) { > + case RV_CTX_F_SEEN_CALL: > + case RV_CTX_F_SEEN_S1: > + case RV_CTX_F_SEEN_S2: > + case RV_CTX_F_SEEN_S3: > + case RV_CTX_F_SEEN_S4: > + case RV_CTX_F_SEEN_S5: > + case RV_CTX_F_SEEN_S6: > + return test_bit(reg, &ctx->flags); > + } > + return false; > +} > + > +static void mark_call(struct rv_jit_context *ctx) > +{ > + __set_bit(RV_CTX_F_SEEN_CALL, &ctx->flags); > +} > + > +static bool seen_call(struct rv_jit_context *ctx) > +{ > + return seen_reg(RV_REG_RA, ctx); > +} Just nit: probably might be more obvious to remove this asymmetry in seen_reg() and do __set_bit()/test_bit() for RV_CTX_F_SEEN_CALL similar like below. > +static void mark_tail_call(struct rv_jit_context *ctx) > +{ > + __set_bit(RV_CTX_F_SEEN_TAIL_CALL, &ctx->flags); > +} > + > +static bool seen_tail_call(struct rv_jit_context *ctx) > +{ > + return test_bit(RV_CTX_F_SEEN_TAIL_CALL, &ctx->flags); > +} > + > +static u8 rv_tail_call_reg(struct rv_jit_context *ctx) > +{ > + mark_tail_call(ctx); > + > + if (seen_call(ctx)) { > + __set_bit(RV_CTX_F_SEEN_S6, &ctx->flags); > + return RV_REG_S6; > + } > + return RV_REG_A6; > +} > + > +static void emit(const u32 insn, struct rv_jit_context *ctx) > +{ > + if (ctx->insns) > + ctx->insns[ctx->ninsns] = insn; > + > + ctx->ninsns++; > +} > + > +static u32 rv_r_insn(u8 funct7, u8 rs2, u8 rs1, u8 funct3, u8 rd, u8 opcode) > +{ [...] > + /* Allocate image, now that we know the size. */ > + image_size = sizeof(u32) * ctx->ninsns; > + jit_data->header = bpf_jit_binary_alloc(image_size, &jit_data->image, > + sizeof(u32), > + bpf_fill_ill_insns); > + if (!jit_data->header) { > + prog = orig_prog; > + goto out_offset; > + } > + > + /* Second, real pass, that acutally emits the image. */ > + ctx->insns = (u32 *)jit_data->image; > +skip_init_ctx: > + ctx->ninsns = 0; > + > + build_prologue(ctx); > + if (build_body(ctx, extra_pass)) { > + bpf_jit_binary_free(jit_data->header); > + prog = orig_prog; > + goto out_offset; > + } > + build_epilogue(ctx); > + > + if (bpf_jit_enable > 1) > + bpf_jit_dump(prog->len, image_size, 2, ctx->insns); > + > + prog->bpf_func = (void *)ctx->insns; > + prog->jited = 1; > + prog->jited_len = image_size; > + > + bpf_flush_icache(jit_data->header, (u8 *)ctx->insns + ctx->ninsns); Shouldn't this be '(u32 *)ctx->insns + ctx->ninsns' to cover the range? > + > + if (!prog->is_func || extra_pass) { > +out_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 ? > + tmp : orig_prog); > + return prog; > +} > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2019-02-04 20:06 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-03 11:51 [PATCH bpf-next 0/3] Add RISC-V (RV64G) BPF JIT bjorn.topel 2019-02-03 11:51 ` bjorn.topel 2019-02-03 11:51 ` [PATCH bpf-next 1/3] bpf, riscv: add BPF JIT for RV64G bjorn.topel 2019-02-03 11:51 ` bjorn.topel 2019-02-03 17:08 ` David Miller 2019-02-03 17:08 ` David Miller 2019-02-04 20:06 ` Daniel Borkmann [this message] 2019-02-04 20:06 ` Daniel Borkmann 2019-02-04 20:27 ` Björn Töpel 2019-02-04 20:27 ` Björn Töpel 2019-02-03 11:51 ` [PATCH bpf-next 2/3] MAINTAINERS: add RISC-V BPF JIT maintainer bjorn.topel 2019-02-03 11:51 ` bjorn.topel 2019-02-03 17:08 ` David Miller 2019-02-03 17:08 ` David Miller 2019-02-03 11:51 ` [PATCH bpf-next 3/3] bpf, doc: add RISC-V to filter.txt bjorn.topel 2019-02-03 11:51 ` bjorn.topel 2019-02-03 17:08 ` David Miller 2019-02-03 17:08 ` David Miller 2019-02-04 20:09 ` Daniel Borkmann 2019-02-04 20:09 ` Daniel Borkmann 2019-02-04 20:16 ` Björn Töpel 2019-02-04 20:16 ` Björn Töpel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=88cdae60-494e-6294-b2c1-10b9cbeb95ac@iogearbox.net \ --to=daniel@iogearbox.net \ --cc=ast@kernel.org \ --cc=bjorn.topel@gmail.com \ --cc=hch@infradead.org \ --cc=linux-riscv@lists.infradead.org \ --cc=netdev@vger.kernel.org \ --cc=palmer@sifive.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.