On Wed, Dec 6, 2023 at 11:34 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Fri, Dec 01, 2023 at 11:06:52AM -0800, Song Liu wrote:
> > +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> > + struct bpf_tramp_links *tlinks, void *func_addr)
> > +{
> > + struct bpf_tramp_image im;
> > + void *image;
> > + int ret;
> > +
> > + /* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
> > + * This will NOT cause fragmentation in direct map, as we do not
> > + * call set_memory_*() on this buffer.
> > + */
> > + image = bpf_jit_alloc_exec(PAGE_SIZE);
> > + if (!image)
> > + return -ENOMEM;
> > +
> > + ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, m, flags,
> > + tlinks, func_addr);
> > + bpf_jit_free_exec(image);
> > + return ret;
> > +}
>
> There is no need to allocate an executable page just to compute the size, right?
> Instead of bpf_jit_alloc_exec() it should work with alloc_page() ?
We can use kvmalloc in patch 7. But we need bpf_jit_alloc_exec(). The reason is
__arch_prepare_bpf_trampoline() assumes "image" falls in certain memory ranges.
If we use kvmalloc here, we may fail those checks, for example is_simm32() in
emit_patch().
In patch 7, we separate rw_image from image, so rw_image do not need to be in
the range.
Thanks,
Song
>
> Similar in patch 7:
> int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void
> *image, void *image_end,
> const struct btf_func_model *m, u32 flags,
> struct bpf_tramp_links *tlinks,
> void *func_addr)
> {
> - return __arch_prepare_bpf_trampoline(im, image, image_end, m,
> flags, tlinks, func_addr);
> + void *rw_image, *tmp;
> + int ret;
> + u32 size = image_end - image;
> +
> + rw_image = bpf_jit_alloc_exec(size);
> + if (!rw_image)
> + return -ENOMEM;
> +
> + ret = __arch_prepare_bpf_trampoline(im, rw_image, rw_image +
> size, image, m,
> + flags, tlinks, func_addr);
> + if (ret < 0)
> + goto out;
> +
> + tmp = bpf_arch_text_copy(image, rw_image, size);
> + if (IS_ERR(tmp))
> + ret = PTR_ERR(tmp);
> +out:
> + bpf_jit_free_exec(rw_image);
> + return ret;
> }
>
> In the above only 'image' has to be ROX. rw_image can be allocated
> with kvmalloc().
> Just like it's done in the main loop of JIT via
> bpf_jit_binary_pack_alloc() -> kvmalloc() -> rw_header.
>
> pw-bot: cr
