> On Sep 27, 2023, at 6:16 AM, Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Tue, Sep 26, 2023 at 12:00:20PM -0700, Song Liu wrote:
>
> SNIP
>
>> @@ -2665,25 +2672,61 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
>> if (flags & BPF_TRAMP_F_SKIP_FRAME)
>> /* skip our return address and return to parent */
>> EMIT4(0x48, 0x83, 0xC4, 8); /* add rsp, 8 */
>> - emit_return(&prog, prog);
>> + emit_return(&prog, image + (prog - (u8 *)rw_image));
>> /* Make sure the trampoline generation logic doesn't overflow */
>> - if (WARN_ON_ONCE(prog > (u8 *)image_end - BPF_INSN_SAFETY)) {
>> + if (WARN_ON_ONCE(prog > (u8 *)rw_image_end - BPF_INSN_SAFETY)) {
>> ret = -EFAULT;
>> goto cleanup;
>> }
>> - ret = prog - (u8 *)image + BPF_INSN_SAFETY;
>> + ret = prog - (u8 *)rw_image + BPF_INSN_SAFETY;
>>
>> cleanup:
>> kfree(branches);
>> return ret;
>> }
>>
>> +void *arch_alloc_bpf_trampoline(int size)
>> +{
>> + return bpf_prog_pack_alloc(size, jit_fill_hole);
>> +}
>> +
>> +void arch_free_bpf_trampoline(void *image, int size)
>> +{
>> + bpf_prog_pack_free(image, size);
>> +}
>> +
>> +void arch_protect_bpf_trampoline(void *image, int size)
>> +{
>> +}
>> +
>> +void arch_unprotect_bpf_trampoline(void *image, int size)
>> +{
>> +}
>
> seems bit confusing having empty non weak functions to overload
> the weak versions IIUC
>
> would maybe some other way fit better than weak functions in here?
> like having arch specific macro to use bpf_prog_pack_alloc for
> trampoline allocation
We can also have a few flags that arch code set at init time.
Then we can use these flags in trampoline.c. But I don't think
that's cleaner than current version.
With more archs adopting bpf_prog_pack, I think we will be able
to remove these helpers soon.
Thanks,
Song
>
> feel free to disregard if you have already investigated this ;-)
