On Tue, Jul 2, 2024 at 2:19 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Tue, 2024-07-02 at 14:09 -0700, Andrii Nakryiko wrote: > > [...] > > > you are defining a general framework with these changes, though, so > > let's introduce a standard and simple way to do this. Say, in addition > > to having arch-specific bpf_jit_inlines_helper_call() we can have > > bpf_jit_supports_helper_nocsr() or something. And they should be > > defined next to each other, so whenever one changes it's easier to > > remember to change the other one. > > > > I don't think requiring arm64 contributors to change the code of > > call_csr_mask() is the right approach. > > I'd change the return value for bpf_jit_inlines_helper_call() to enum, > to avoid naming functions several times. Hm... I don't know, feels ugly. nocsr is sort of a dependent but orthogonal axis, so I'd keep it separate. Imagine we add yet another property that relies on inlining, but is independent of nocsr. I guess we can have enum be a bit set, but I'd start simple with two functions. > > [...] > > > > > strictly speaking, does nocsr have anything to do with inlining, > > > > though? E.g., if we know for sure (however, that's a separate issue) > > > > that helper implementation doesn't touch extra registers, why do we > > > > need inlining to make use of nocsr? > > > > > > Technically, alternative for nocsr is for C version of the > > > helper/kfunc itself has no_caller_saved_registers attribute. > > > Grep shows a single function annotated as such in kernel tree: > > > stackleak_track_stack(). > > > Or, maybe, for helpers/kfuncs implemented in assembly. > > > > Yes, I suppose it's too dangerous to rely on the compiler to not use > > some extra register. I guess worst case we can "inline" helper by > > keeping call to it intact :) > > Something like that. > > [...]
