Eduard Zingerman <eddyz87@xxxxxxxxx> writes: > On Wed, 2024-07-03 at 11:57 +0000, Puranjay Mohan wrote: > > [...] > >> > +static u32 call_csr_mask(struct bpf_verifier_env *env, struct bpf_insn *insn) >> > +{ >> > + const struct bpf_func_proto *fn; >> > + >> > + if (bpf_helper_call(insn) && >> > + verifier_inlines_helper_call(env, insn->imm) && >> >> This should also check bpf_jit_inlines_helper_call(insn->imm) as the JIT >> can also inline helper calls separately from the verifier. >> >> if (bpf_helper_call(insn) && >> (verifier_inlines_helper_call(env, insn->imm) || bpf_jit_inlines_helper_call(insn->imm)) && >> >> This is currently being done by the arm64 and risc-v JITs and they don't >> scratch any register except R0 (The helpers inlined by these JITs are >> non-void). > > Hello Puranjay, thank you for commenting. > In a sibling email Andrii suggested to also add a function like below: > > __weak bool bpf_jit_supports_helper_nocsr(s32) > > At the moment I see the following helpers inlined by jits: > - arm64: > - BPF_FUNC_get_smp_processor_id > - BPF_FUNC_get_current_task > - BPF_FUNC_get_current_task_btf > - riscv: > - BPF_FUNC_get_smp_processor_id Yes, all of the above conform to nocsr. > I suspect (but need to double check) that all of these patches conform > to nocsr. If so, what are you thoughts regarding bpf_jit_supports_helper_nocsr(): > do we need it, or should inlining imply nocsr? I don't think we need bpf_jit_supports_helper_nocsr() because JITs will be only inlining very simple helpers and will not clobber caller saved registers. JITs anyway use temperory registers for intermediate steps if required. Thanks, Puranjay
Attachment:
signature.asc
Description: PGP signature