On Sun, Feb 05, 2023 at 07:36:14PM +0100, Ilya Leoshkevich wrote: > On Sun, 2023-02-05 at 19:17 +0100, Jiri Olsa wrote: > > On Sat, Feb 04, 2023 at 01:21:13AM -0800, Alexei Starovoitov wrote: > > > On Fri, Feb 3, 2023 at 8:23 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > > > > > > > hi, > > > > I noticed several times in discussions that we should move test > > > > kfuncs > > > > into kernel module, now perhaps even more pressing with all the > > > > kfunc > > > > effort. This patchset moves all the test kfuncs into bpf_testmod. > > > > > > > > I added bpf_testmod/bpf_testmod_kfunc.h header that is shared > > > > between > > > > bpf_testmod kernel module and BPF programs, which brings some > > > > difficulties > > > > with __ksym define. But I'm not sure having separate headers for > > > > BPF > > > > programs and for kernel module would be better. > > > > > > > > This patchset also needs: > > > > 74bc3a5acc82 bpf: Add missing btf_put to > > > > register_btf_id_dtor_kfuncs > > > > which is only in bpf/master now. > > > > > > I thought you've added this patch to CI, > > > but cb_refs is still failing on s390... > > > > the CI now fails for s390 with messages like: > > 2023-02-04T07:04:32.5185267Z RES: address of kernel function > > bpf_kfunc_call_test_fail1 is out of range > > > > so now that we have test kfuncs in the module, the 's32 imm' value of > > the bpf call instructions can overflow when the offset between module > > and kernel is greater than 2GB ... as explained in the commit that > > added the verifier check: > > > > 8cbf062a250e bpf: Reject kfunc calls that overflow insn->imm > > > > not sure we can do anything about that on bpf side.. cc-ing s390 list > > and Ilya for ideas/thoughts > > > > maybe we could make bpf_testmod in-tree module and compile it as > > module > > just for some archs > > > > thoughts? > > Hi, > > I'd rather have this fixed - I guess the problem can affect the users. > The ksyms_module test is already denylisted because of that. > Unfortunately getting the kernel and the modules close together on > s390x is unlikely to happen in the foreseeable future. > > What do you think about keeping the BTF ID inside the insn->imm field > and putting the 64-bit delta into bpf_insn_aux_data, replacing the > call_imm field that we already have there? seems tricky wrt other archs.. how about saving address of the kfunc in bpf_insn_aux_data and use that in s390 jit code instead of the '__bpf_call_base + imm' calculation jirka
