[PATCH v2] powerpc64/bpf: fix JIT code size calculation of bpf trampoline
Hari Bathini
hbathini at linux.ibm.com
Tue Apr 22 18:29:43 AEST 2025
On 19/04/25 4:31 pm, Naveen N Rao wrote:
> On Thu, Apr 17, 2025 at 01:10:37AM +0530, Hari Bathini wrote:
>> arch_bpf_trampoline_size() provides JIT size of the BPF trampoline
>> before the buffer for JIT'ing it is allocated. The total number of
>> instructions emitted for BPF trampoline JIT code depends on where
>> the final image is located. So, the size arrived at with the dummy
>> pass in arch_bpf_trampoline_size() can vary from the actual size
>> needed in arch_prepare_bpf_trampoline(). When the instructions
>> accounted in arch_bpf_trampoline_size() is less than the number of
>> instructions emitted during the actual JIT compile of the trampoline,
>> the below warning is produced:
>>
>> WARNING: CPU: 8 PID: 204190 at arch/powerpc/net/bpf_jit_comp.c:981 __arch_prepare_bpf_trampoline.isra.0+0xd2c/0xdcc
>>
>> which is:
>>
>> /* Make sure the trampoline generation logic doesn't overflow */
>> if (image && WARN_ON_ONCE(&image[ctx->idx] >
>> (u32 *)rw_image_end - BPF_INSN_SAFETY)) {
>>
>> So, during the dummy pass, instead of providing some arbitrary image
>> location, account for maximum possible instructions if and when there
>> is a dependency with image location for JIT'ing.
>>
>> Fixes: d243b62b7bd3 ("powerpc64/bpf: Add support for bpf trampolines")
>> Reported-by: Venkat Rao Bagalkote <venkat88 at linux.ibm.com>
>> Closes: https://lore.kernel.org/all/6168bfc8-659f-4b5a-a6fb-90a916dde3b3@linux.ibm.com/
>> Cc: stable at vger.kernel.org # v6.13+
>> Signed-off-by: Hari Bathini <hbathini at linux.ibm.com>
>> ---
>>
>> Changes since v1:
>> - Pass NULL for image during intial pass and account for max. possible
>> instruction during this pass as Naveen suggested.
>>
>>
>> arch/powerpc/net/bpf_jit.h | 20 ++++++++++++++++---
>> arch/powerpc/net/bpf_jit_comp.c | 33 ++++++++++---------------------
>> arch/powerpc/net/bpf_jit_comp64.c | 9 +++++++++
>> 3 files changed, 36 insertions(+), 26 deletions(-)
>>
>> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
>> index 6beacaec63d3..4c26912c2e3c 100644
>> --- a/arch/powerpc/net/bpf_jit.h
>> +++ b/arch/powerpc/net/bpf_jit.h
>> @@ -51,8 +51,16 @@
>> EMIT(PPC_INST_BRANCH_COND | (((cond) & 0x3ff) << 16) | (offset & 0xfffc)); \
>> } while (0)
>>
>> -/* Sign-extended 32-bit immediate load */
>> +/*
>> + * Sign-extended 32-bit immediate load
>> + *
>> + * If this is a dummy pass (!image), account for
>> + * maximum possible instructions.
>> + */
>> #define PPC_LI32(d, i) do { \
>> + if (!image) \
>> + ctx->idx += 2; \
>> + else { \
>> if ((int)(uintptr_t)(i) >= -32768 && \
>> (int)(uintptr_t)(i) < 32768) \
>> EMIT(PPC_RAW_LI(d, i)); \
>> @@ -60,10 +68,15 @@
>> EMIT(PPC_RAW_LIS(d, IMM_H(i))); \
>> if (IMM_L(i)) \
>> EMIT(PPC_RAW_ORI(d, d, IMM_L(i))); \
>> - } } while(0)
>> + } \
>> + } } while (0)
>>
>> #ifdef CONFIG_PPC64
>> +/* If dummy pass (!image), account for maximum possible instructions */
>> #define PPC_LI64(d, i) do { \
>> + if (!image) \
>> + ctx->idx += 5; \
>> + else { \
>> if ((long)(i) >= -2147483648 && \
>> (long)(i) < 2147483648) \
>> PPC_LI32(d, i); \
>> @@ -84,7 +97,8 @@
>> if ((uintptr_t)(i) & 0x000000000000ffffULL) \
>> EMIT(PPC_RAW_ORI(d, d, (uintptr_t)(i) & \
>> 0xffff)); \
>> - } } while (0)
>> + } \
>> + } } while (0)
>
> You should now also be able to remove the padding we add in
> bpf_jit_comp64.c for 'case BPF_LD | BPF_IMM | BPF_DW:'
>
>> #define PPC_LI_ADDR PPC_LI64
>>
>> #ifndef CONFIG_PPC_KERNEL_PCREL
>> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
>> index 2991bb171a9b..c0684733e9d6 100644
>> --- a/arch/powerpc/net/bpf_jit_comp.c
>> +++ b/arch/powerpc/net/bpf_jit_comp.c
>> @@ -504,10 +504,11 @@ static int invoke_bpf_prog(u32 *image, u32 *ro_image, struct codegen_context *ct
>> EMIT(PPC_RAW_ADDI(_R3, _R1, regs_off));
>> if (!p->jited)
>> PPC_LI_ADDR(_R4, (unsigned long)p->insnsi);
>> - if (!create_branch(&branch_insn, (u32 *)&ro_image[ctx->idx], (unsigned long)p->bpf_func,
>> - BRANCH_SET_LINK)) {
>> - if (image)
>> - image[ctx->idx] = ppc_inst_val(branch_insn);
>> + /* Account for max possible instructions during dummy pass for size calculation */
>> + if (image && !create_branch(&branch_insn, (u32 *)&ro_image[ctx->idx],
>> + (unsigned long)p->bpf_func,
>> + BRANCH_SET_LINK)) {
>> + image[ctx->idx] = ppc_inst_val(branch_insn);
>> ctx->idx++;
>> } else {
>> EMIT(PPC_RAW_LL(_R12, _R25, offsetof(struct bpf_prog, bpf_func)));
>> @@ -889,7 +890,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
>> bpf_trampoline_restore_tail_call_cnt(image, ctx, func_frame_offset, r4_off);
>>
>> /* Reserve space to patch branch instruction to skip fexit progs */
>> - im->ip_after_call = &((u32 *)ro_image)[ctx->idx];
>> + if (ro_image) /* image is NULL for dummy pass */
>> + im->ip_after_call = &((u32 *)ro_image)[ctx->idx];
>> EMIT(PPC_RAW_NOP());
>> }
>>
>> @@ -912,7 +914,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
>> }
>>
>> if (flags & BPF_TRAMP_F_CALL_ORIG) {
>> - im->ip_epilogue = &((u32 *)ro_image)[ctx->idx];
>> + if (ro_image) /* image is NULL for dummy pass */
>> + im->ip_epilogue = &((u32 *)ro_image)[ctx->idx];
>> PPC_LI_ADDR(_R3, im);
>> ret = bpf_jit_emit_func_call_rel(image, ro_image, ctx,
>> (unsigned long)__bpf_tramp_exit);
>> @@ -973,25 +976,9 @@ 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.
>> - *
>> - * We cannot use kvmalloc here, because we need image to be in
>> - * module memory range.
>> - */
>> - image = bpf_jit_alloc_exec(PAGE_SIZE);
>> - if (!image)
>> - return -ENOMEM;
>> -
>> - ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, image,
>> - m, flags, tlinks, func_addr);
>> - bpf_jit_free_exec(image);
>> -
>> + ret = __arch_prepare_bpf_trampoline(&im, NULL, NULL, NULL, m, flags, tlinks, func_addr);
>> return ret;
>> }
>>
>> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
>> index 233703b06d7c..91f9efe8b8d7 100644
>> --- a/arch/powerpc/net/bpf_jit_comp64.c
>> +++ b/arch/powerpc/net/bpf_jit_comp64.c
>> @@ -225,6 +225,15 @@ int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *
>> }
>>
>> #ifdef CONFIG_PPC_KERNEL_PCREL
>> + /*
>> + * If fimage is NULL (the initial pass to find image size),
>> + * account for the maximum no. of instructions possible.
>> + */
>> + if (!fimage) {
>> + ctx->idx += 7;
>> + return 0;
>> + }
>> +
>
> I would merge this with the below if conditional so that this gets
> noticed if the instruction sequence below ever changes.
>
>> reladdr = func_addr - local_paca->kernelbase;
>>
>> if (reladdr < (long)SZ_8G && reladdr >= -(long)SZ_8G) {
>
> Other than that:
> Acked-by: Naveen N Rao (AMD) <naveen at kernel.org>
Thanks for the review, Naveen.
Posted v3 with the suggested change.
https://lore.kernel.org/linuxppc-dev/20250422082609.949301-1-hbathini@linux.ibm.com/
- Hari
More information about the Linuxppc-dev
mailing list