[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ+HfNghhE37mAWvfUQaTfnzHktYHd+XYq3SMLJcY19Qbsz7Ww@mail.gmail.com>
Date: Wed, 15 Jul 2020 20:02:36 +0200
From: Björn Töpel <bjorn.topel@...il.com>
To: Luke Nelson <lukenels@...washington.edu>
Cc: bpf <bpf@...r.kernel.org>, Luke Nelson <luke.r.nels@...il.com>,
Xi Wang <xi.wang@...il.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
Andrii Nakryiko <andriin@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>,
Netdev <netdev@...r.kernel.org>,
linux-riscv <linux-riscv@...ts.infradead.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH bpf-next 1/3] bpf, riscv: Modify JIT ctx to support
compressed instructions
On Mon, 13 Jul 2020 at 20:37, Luke Nelson <lukenels@...washington.edu> wrote:
>
> This patch makes the necessary changes to struct rv_jit_context and to
> bpf_int_jit_compile to support compressed riscv (RVC) instructions in
> the BPF JIT.
>
> It changes the JIT image to be u16 instead of u32, since RVC instructions
> are 2 bytes as opposed to 4.
>
> It also changes ctx->offset and ctx->ninsns to refer to 2-byte
> instructions rather than 4-byte ones. The riscv PC is always 16-bit
> aligned, so this is sufficient to refer to any valid riscv offset.
>
> The code for computing jump offsets in bytes is updated accordingly,
> and factored in to the RVOFF macro to simplify the code.
>
> Signed-off-by: Luke Nelson <luke.r.nels@...il.com>
> ---
> arch/riscv/net/bpf_jit.h | 23 ++++++++++++++++++++---
> arch/riscv/net/bpf_jit_comp32.c | 14 +++++++-------
> arch/riscv/net/bpf_jit_comp64.c | 12 ++++++------
> arch/riscv/net/bpf_jit_core.c | 6 +++---
> 4 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
> index 20e235d06f66..5c89ea904c1a 100644
> --- a/arch/riscv/net/bpf_jit.h
> +++ b/arch/riscv/net/bpf_jit.h
> @@ -50,7 +50,7 @@ enum {
>
> struct rv_jit_context {
> struct bpf_prog *prog;
> - u32 *insns; /* RV insns */
> + u16 *insns; /* RV insns */
> int ninsns;
> int epilogue_offset;
> int *offset; /* BPF to RV */
> @@ -58,6 +58,9 @@ struct rv_jit_context {
> int stack_size;
> };
>
> +/* Convert from ninsns to bytes. */
> +#define RVOFF(ninsns) ((ninsns) << 1)
> +
I guess it's a matter of taste, but I'd prefer a simple static inline
function instead of the macro.
> struct rv_jit_data {
> struct bpf_binary_header *header;
> u8 *image;
> @@ -74,8 +77,22 @@ static inline void bpf_flush_icache(void *start, void *end)
> flush_icache_range((unsigned long)start, (unsigned long)end);
> }
>
> +/* Emit a 4-byte riscv instruction. */
> static inline void emit(const u32 insn, struct rv_jit_context *ctx)
> {
> + if (ctx->insns) {
> + ctx->insns[ctx->ninsns] = insn;
> + ctx->insns[ctx->ninsns + 1] = (insn >> 16);
> + }
> +
> + ctx->ninsns += 2;
> +}
> +
> +/* Emit a 2-byte riscv compressed instruction. */
> +static inline void emitc(const u16 insn, struct rv_jit_context *ctx)
> +{
> + BUILD_BUG_ON(!rvc_enabled());
> +
> if (ctx->insns)
> ctx->insns[ctx->ninsns] = insn;
>
> @@ -86,7 +103,7 @@ static inline int epilogue_offset(struct rv_jit_context *ctx)
> {
> int to = ctx->epilogue_offset, from = ctx->ninsns;
>
> - return (to - from) << 2;
> + return RVOFF(to - from);
> }
>
> /* Return -1 or inverted cond. */
> @@ -149,7 +166,7 @@ static inline int rv_offset(int insn, int off, struct rv_jit_context *ctx)
> off++; /* BPF branch is from PC+1, RV is from PC */
> from = (insn > 0) ? ctx->offset[insn - 1] : 0;
> to = (insn + off > 0) ? ctx->offset[insn + off - 1] : 0;
> - return (to - from) << 2;
> + return RVOFF(to - from);
> }
>
> /* Instruction formats. */
> diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
> index b198eaa74456..d22001aa0057 100644
> --- a/arch/riscv/net/bpf_jit_comp32.c
> +++ b/arch/riscv/net/bpf_jit_comp32.c
> @@ -644,7 +644,7 @@ static int emit_branch_r64(const s8 *src1, const s8 *src2, s32 rvoff,
>
> e = ctx->ninsns;
> /* Adjust for extra insns. */
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
> emit_jump_and_link(RV_REG_ZERO, rvoff, true, ctx);
> return 0;
> }
> @@ -713,7 +713,7 @@ static int emit_bcc(u8 op, u8 rd, u8 rs, int rvoff, struct rv_jit_context *ctx)
> if (far) {
> e = ctx->ninsns;
> /* Adjust for extra insns. */
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
> emit_jump_and_link(RV_REG_ZERO, rvoff, true, ctx);
> }
> return 0;
> @@ -731,7 +731,7 @@ static int emit_branch_r32(const s8 *src1, const s8 *src2, s32 rvoff,
>
> e = ctx->ninsns;
> /* Adjust for extra insns. */
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
>
> if (emit_bcc(op, lo(rs1), lo(rs2), rvoff, ctx))
> return -1;
> @@ -795,7 +795,7 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
> * if (index >= max_entries)
> * goto out;
> */
> - off = (tc_ninsn - (ctx->ninsns - start_insn)) << 2;
> + off = RVOFF(tc_ninsn - (ctx->ninsns - start_insn));
> emit_bcc(BPF_JGE, lo(idx_reg), RV_REG_T1, off, ctx);
>
> /*
> @@ -804,7 +804,7 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
> * goto out;
> */
> emit(rv_addi(RV_REG_T1, RV_REG_TCC, -1), ctx);
> - off = (tc_ninsn - (ctx->ninsns - start_insn)) << 2;
> + off = RVOFF(tc_ninsn - (ctx->ninsns - start_insn));
> emit_bcc(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
>
> /*
> @@ -818,7 +818,7 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
> if (is_12b_check(off, insn))
> return -1;
> emit(rv_lw(RV_REG_T0, off, RV_REG_T0), ctx);
> - off = (tc_ninsn - (ctx->ninsns - start_insn)) << 2;
> + off = RVOFF(tc_ninsn - (ctx->ninsns - start_insn));
> emit_bcc(BPF_JEQ, RV_REG_T0, RV_REG_ZERO, off, ctx);
>
> /*
> @@ -1214,7 +1214,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
> emit_imm32(tmp2, imm, ctx);
> src = tmp2;
> e = ctx->ninsns;
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
> }
>
> if (is64)
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 6cfd164cbe88..26feed92f1bc 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -304,14 +304,14 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
> if (is_12b_check(off, insn))
> return -1;
> emit(rv_lwu(RV_REG_T1, off, RV_REG_A1), ctx);
> - off = (tc_ninsn - (ctx->ninsns - start_insn)) << 2;
> + off = RVOFF(tc_ninsn - (ctx->ninsns - start_insn));
> emit_branch(BPF_JGE, RV_REG_A2, RV_REG_T1, off, ctx);
>
> /* if (TCC-- < 0)
> * goto out;
> */
> emit(rv_addi(RV_REG_T1, tcc, -1), ctx);
> - off = (tc_ninsn - (ctx->ninsns - start_insn)) << 2;
> + off = RVOFF(tc_ninsn - (ctx->ninsns - start_insn));
> emit_branch(BPF_JSLT, tcc, RV_REG_ZERO, off, ctx);
>
> /* prog = array->ptrs[index];
> @@ -324,7 +324,7 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
> if (is_12b_check(off, insn))
> return -1;
> emit(rv_ld(RV_REG_T2, off, RV_REG_T2), ctx);
> - off = (tc_ninsn - (ctx->ninsns - start_insn)) << 2;
> + off = RVOFF(tc_ninsn - (ctx->ninsns - start_insn));
> emit_branch(BPF_JEQ, RV_REG_T2, RV_REG_ZERO, off, ctx);
>
> /* goto *(prog->bpf_func + 4); */
> @@ -757,7 +757,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
> e = ctx->ninsns;
>
> /* Adjust for extra insns */
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
> }
>
> if (BPF_OP(code) == BPF_JSET) {
> @@ -810,7 +810,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
> e = ctx->ninsns;
>
> /* Adjust for extra insns */
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
> emit_branch(BPF_OP(code), rd, rs, rvoff, ctx);
> break;
>
> @@ -831,7 +831,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
> if (!is64 && imm < 0)
> emit(rv_addiw(RV_REG_T1, RV_REG_T1, 0), ctx);
> e = ctx->ninsns;
> - rvoff -= (e - s) << 2;
> + rvoff -= RVOFF(e - s);
> emit_branch(BPF_JNE, RV_REG_T1, RV_REG_ZERO, rvoff, ctx);
> break;
>
> diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
> index 709b94ece3ed..cd156efe4944 100644
> --- a/arch/riscv/net/bpf_jit_core.c
> +++ b/arch/riscv/net/bpf_jit_core.c
> @@ -73,7 +73,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>
> if (ctx->offset) {
> extra_pass = true;
> - image_size = sizeof(u32) * ctx->ninsns;
> + image_size = sizeof(u16) * ctx->ninsns;
Maybe sizeof(*ctx->insns)?
> goto skip_init_ctx;
> }
>
> @@ -103,7 +103,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> if (jit_data->header)
> break;
>
> - image_size = sizeof(u32) * ctx->ninsns;
> + image_size = sizeof(u16) * ctx->ninsns;
Dito.
Björn
> jit_data->header =
> bpf_jit_binary_alloc(image_size,
> &jit_data->image,
> @@ -114,7 +114,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> goto out_offset;
> }
>
> - ctx->insns = (u32 *)jit_data->image;
> + ctx->insns = (u16 *)jit_data->image;
> /*
> * Now, when the image is allocated, the image can
> * potentially shrink more (auipc/jalr -> jal).
> --
> 2.25.1
>
Powered by blists - more mailing lists