[PATCH v1 06/10] powerpc/bpf: Perform complete extra passes to update addresses

[Christophe Leroy]

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 at 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.

My understanding is that bpf_int_jit_compile() is called twice by 
jit_subprogs(), second call sets 'extra_pass" due to jit_data->addrs = 
addrs being set at the end of first pass.

Christophe