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

[Puranjay Mohan <puranjay@kernel.org>]

[-- 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 --]