Re: [RFC bpf-next v1 2/8] bpf: no_caller_saved_registers attribute for helper calls

Puranjay Mohan
2024-07-04T10:55:42Z

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

Eduard Zingerman <eddyz87@gmail.com> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]