lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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