Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230607091814.46080-1-puranjay12@gmail.com> <20230607091814.46080-4-puranjay12@gmail.com>
In-Reply-To: <20230607091814.46080-4-puranjay12@gmail.com>
From:   Song Liu <song@kernel.org>
Date:   Thu, 8 Jun 2023 09:28:25 -0700
Message-ID: <CAPhsuW7soOQasGw5fHB2qTeJnqR4ZrGBodyO87k=vg=TYqCsWA@mail.gmail.com>
Subject: Re: [PATCH bpf-next v2 3/3] bpf, arm64: use bpf_jit_binary_pack_alloc
To:     Puranjay Mohan <puranjay12@gmail.com>
Cc:     ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
        martin.lau@linux.dev, catalin.marinas@arm.com,
        mark.rutland@arm.com, bpf@vger.kernel.org, kpsingh@kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jun 7, 2023 at 2:18=E2=80=AFAM Puranjay Mohan <puranjay12@gmail.com=
> wrote:
>
[...]
> +
>  static inline int epilogue_offset(const struct jit_ctx *ctx)
>  {
>         int to =3D ctx->epilogue_offset;
> @@ -701,7 +716,8 @@ static int add_exception_handler(const struct bpf_ins=
n *insn,
>                                  struct jit_ctx *ctx,
>                                  int dst_reg)
>  {
> -       off_t offset;
> +       off_t ins_offset;
> +       off_t fixup_offset;

Please add some comments for these two offsets.

>         unsigned long pc;
>         struct exception_table_entry *ex;
>
> @@ -717,12 +733,11 @@ static int add_exception_handler(const struct bpf_i=
nsn *insn,
>                 return -EINVAL;
>
>         ex =3D &ctx->prog->aux->extable[ctx->exentry_idx];
> -       pc =3D (unsigned long)&ctx->image[ctx->idx - 1];
> +       pc =3D (unsigned long)&ctx->ro_image[ctx->idx - 1];
>
> -       offset =3D pc - (long)&ex->insn;
> -       if (WARN_ON_ONCE(offset >=3D 0 || offset < INT_MIN))
> +       ins_offset =3D pc - (long)&ex->insn;
> +       if (WARN_ON_ONCE(ins_offset >=3D 0 || ins_offset < INT_MIN))
>                 return -ERANGE;
> -       ex->insn =3D offset;
>
>         /*
>          * Since the extable follows the program, the fixup offset is alw=
ays
> @@ -732,11 +747,20 @@ static int add_exception_handler(const struct bpf_i=
nsn *insn,
>          * modifying the upper bits because the table is already sorted, =
and
>          * isn't part of the main exception table.
>          */
> -       offset =3D (long)&ex->fixup - (pc + AARCH64_INSN_SIZE);
> -       if (!FIELD_FIT(BPF_FIXUP_OFFSET_MASK, offset))
> +       fixup_offset =3D (long)&ex->fixup - (pc + AARCH64_INSN_SIZE);
> +       if (!FIELD_FIT(BPF_FIXUP_OFFSET_MASK, fixup_offset))
>                 return -ERANGE;
>
> -       ex->fixup =3D FIELD_PREP(BPF_FIXUP_OFFSET_MASK, offset) |
> +       /*
> +        * The offsets above have been calculated using the RO buffer but=
 we
> +        * need to use the R/W buffer for writes.
> +        * switch ex to rw buffer for writing.
> +        */
> +       ex =3D (void *)ctx->image + ((void *)ex - (void *)ctx->ro_image);
> +
> +       ex->insn =3D ins_offset;
> +
> +       ex->fixup =3D FIELD_PREP(BPF_FIXUP_OFFSET_MASK, fixup_offset) |
>                     FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
>
>         ex->type =3D EX_TYPE_BPF;
[...]
>         /* And we're done. */
>         if (bpf_jit_enable > 1)
>                 bpf_jit_dump(prog->len, prog_size, 2, ctx.image);
>
> -       bpf_flush_icache(header, ctx.image + ctx.idx);
> +       bpf_flush_icache(ro_header, ctx.ro_image + ctx.idx);
>
>         if (!prog->is_func || extra_pass) {
>                 if (extra_pass && ctx.idx !=3D jit_data->ctx.idx) {
>                         pr_err_once("multi-func JIT bug %d !=3D %d\n",
>                                     ctx.idx, jit_data->ctx.idx);
> -                       bpf_jit_binary_free(header);
>                         prog->bpf_func =3D NULL;
>                         prog->jited =3D 0;
>                         prog->jited_len =3D 0;
> +                       goto out_free_hdr;
> +               }
> +               if (WARN_ON(bpf_jit_binary_pack_finalize(prog, ro_header,
> +                                                        header))) {
> +                       ro_header =3D NULL;

I think we need
       prog =3D orig_prog;
here.

Thanks,
Song