Re: [PATCH v4 3/5] powerpc/bpf: use bpf_jit_binary_pack_[alloc|finalize|free]

[Song Liu <song@kernel.org>]

On Fri, Sep 8, 2023 at 6:28 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>
> Use bpf_jit_binary_pack_alloc in powerpc jit. The jit engine first
> writes the program to the rw buffer. When the jit is done, the program
> is copied to the final location with bpf_jit_binary_pack_finalize.
> With multiple jit_subprogs, bpf_jit_free is called on some subprograms
> that haven't got bpf_jit_binary_pack_finalize() yet. Implement custom
> bpf_jit_free() like in commit 1d5f82d9dd47 ("bpf, x86: fix freeing of
> not-finalized bpf_prog_pack") to call bpf_jit_binary_pack_finalize(),
> if necessary. While here, correct the misnomer powerpc64_jit_data to
> powerpc_jit_data as it is meant for both ppc32 and ppc64.

I would personally prefer to put the rename to a separate patch.

>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/net/bpf_jit.h        |  12 ++--
>  arch/powerpc/net/bpf_jit_comp.c   | 110 ++++++++++++++++++++++--------
>  arch/powerpc/net/bpf_jit_comp32.c |  13 ++--
>  arch/powerpc/net/bpf_jit_comp64.c |  10 +--
>  4 files changed, 98 insertions(+), 47 deletions(-)

[...]

> @@ -220,17 +237,19 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>
>  #ifdef CONFIG_PPC64_ELF_ABI_V1
>         /* Function descriptor nastiness: Address + TOC */
> -       ((u64 *)image)[0] = (u64)code_base;
> +       ((u64 *)image)[0] = (u64)fcode_base;
>         ((u64 *)image)[1] = local_paca->kernel_toc;
>  #endif
>
> -       fp->bpf_func = (void *)image;
> +       fp->bpf_func = (void *)fimage;
>         fp->jited = 1;
>         fp->jited_len = proglen + FUNCTION_DESCR_SIZE;
>
> -       bpf_flush_icache(bpf_hdr, (u8 *)bpf_hdr + bpf_hdr->size);

I guess we don't need bpf_flush_icache() any more? So can we remove it
from arch/powerpc/net/bpf_jit.h?

Thanks,
Song

>         if (!fp->is_func || extra_pass) {
> -               bpf_jit_binary_lock_ro(bpf_hdr);
> +               if (bpf_jit_binary_pack_finalize(fp, fhdr, hdr)) {
> +                       fp = org_fp;
> +                       goto out_addrs;
> +               }
>                 bpf_prog_fill_jited_linfo(fp, addrs);
>  out_addrs:
>                 kfree(addrs);
> @@ -240,8 +259,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>                 jit_data->addrs = addrs;
>                 jit_data->ctx = cgctx;
>                 jit_data->proglen = proglen;
> -               jit_data->image = image;
> -               jit_data->header = bpf_hdr;
> +               jit_data->fimage = fimage;
> +               jit_data->fhdr = fhdr;
> +               jit_data->hdr = hdr;
>         }
>
>  out:
[...]