Messages in this thread | | | Date | Tue, 10 Jan 2023 14:14:17 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [PATCH v1 06/10] powerpc/bpf: Perform complete extra passes to update addresses |
| |
Christophe Leroy wrote: > > > Le 13/12/2022 à 11:23, Naveen N. Rao a écrit : >> Christophe Leroy wrote: >>> BPF core calls the jit compiler again for an extra pass in order >>> to properly set subprog addresses. >>> >>> Unlike other architectures, powerpc only updates the addresses >>> during that extra pass. It means that holes must have been left >>> in the code in order to enable the maximum possible instruction >>> size. >>> >>> In order avoid waste of space, and waste of CPU time on powerpc >>> processors on which the NOP instruction is not 0-cycle, perform >>> two real additional passes. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> --- >>> arch/powerpc/net/bpf_jit_comp.c | 85 --------------------------------- >>> 1 file changed, 85 deletions(-) >>> >>> diff --git a/arch/powerpc/net/bpf_jit_comp.c >>> b/arch/powerpc/net/bpf_jit_comp.c >>> index 43e634126514..8833bf23f5aa 100644 >>> --- a/arch/powerpc/net/bpf_jit_comp.c >>> +++ b/arch/powerpc/net/bpf_jit_comp.c >>> @@ -23,74 +23,6 @@ static void bpf_jit_fill_ill_insns(void *area, >>> unsigned int size) >>> memset32(area, BREAKPOINT_INSTRUCTION, size / 4); >>> } >>> >>> -/* Fix updated addresses (for subprog calls, ldimm64, et al) during >>> extra pass */ >>> -static int bpf_jit_fixup_addresses(struct bpf_prog *fp, u32 *image, >>> - struct codegen_context *ctx, u32 *addrs) >>> -{ >>> - const struct bpf_insn *insn = fp->insnsi; >>> - bool func_addr_fixed; >>> - u64 func_addr; >>> - u32 tmp_idx; >>> - int i, j, ret; >>> - >>> - for (i = 0; i < fp->len; i++) { >>> - /* >>> - * During the extra pass, only the branch target addresses for >>> - * the subprog calls need to be fixed. All other instructions >>> - * can left untouched. >>> - * >>> - * The JITed image length does not change because we already >>> - * ensure that the JITed instruction sequence for these calls >>> - * are of fixed length by padding them with NOPs. >>> - */ >>> - if (insn[i].code == (BPF_JMP | BPF_CALL) && >>> - insn[i].src_reg == BPF_PSEUDO_CALL) { >>> - ret = bpf_jit_get_func_addr(fp, &insn[i], true, >>> - &func_addr, >>> - &func_addr_fixed); >> >> I don't see you updating calls to bpf_jit_get_func_addr() in >> bpf_jit_build_body() to set extra_pass to true. Afaics, that's required >> to get the correct address to be branched to for subprogs. >> > > I don't understand what you mean.
I am referring to the third parameter we pass to bpf_jit_get_func_addr().
In bpf_jit_build_body(), we do:
case BPF_JMP | BPF_CALL: ctx->seen |= SEEN_FUNC;
ret = bpf_jit_get_func_addr(fp, &insn[i], false, &func_addr, &func_addr_fixed);
The third parameter (extra_pass) to bpf_jit_get_func_addr() is set to false. In bpf_jit_get_func_addr(), we have:
*func_addr_fixed = insn->src_reg != BPF_PSEUDO_CALL; if (!*func_addr_fixed) { /* Place-holder address till the last pass has collected * all addresses for JITed subprograms in which case we * can pick them up from prog->aux. */ if (!extra_pass) addr = NULL;
Before this patch series, in bpf_jit_fixup_addresses(), we were calling bpf_jit_get_func_addr() with the third parameter set to true.
- Naveen
| |