On Sat, Feb 03, 2024 at 03:40:24PM +0100, Jiri Olsa wrote:
On Fri, Feb 02, 2024 at 03:09:05PM -0800, Manu Bretelle wrote:
On Sun, Jan 28, 2024 at 06:24:05PM -0700, Daniel Xu wrote:
=== Description ===
This is a bpf-treewide change that annotates all kfuncs as such inside .BTF_ids. This annotation eventually allows us to automatically generate kfunc prototypes from bpftool.
We store this metadata inside a yet-unused flags field inside struct btf_id_set8 (thanks Kumar!). pahole will be taught where to look.
More details about the full chain of events are available in commit 3's description.
The accompanying pahole and bpftool changes can be viewed here on these "frozen" branches [0][1].
I hit a similar issue to [0] on master 943b043aeecc ("selftests/bpf: Fix bench runner SIGSEGV") when cross-compiling on x86_64 (LE) to s390x (BE). I do have CONFIG_DEBUG_INFO_BTF enable and the issue would not trigger if I disabled CONFIG_DEBUG_INFO_BTF (and with the fix mentioned in [0]).
What seems to happen is that `tools/resolve_btfids` is ran in the context of the host endianess and if I printk before the WARN_ON: diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index ef380e546952..a9ed7a1a4936 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -8128,6 +8128,7 @@ int register_btf_kfunc_id_set(enum bpf_prog_type prog_type, * WARN() for initcall registrations that do not check errors. */ if (!(kset->set->flags & BTF_SET8_KFUNCS)) {
printk("Flag 0x%08X, expected 0x%08X\n", kset->set->flags, BTF_SET8_KFUNCS); WARN_ON(!kset->owner); return -EINVAL; }the boot logs would show: Flag 0x01000000, expected 0x00000001
The issue did not happen prior to 6f3189f38a3e ("bpf: treewide: Annotate BPF kfuncs in BTF") has only 0 was written before.
It seems [1] will be addressing cross-compilation, but it did not fix it as is by just applying on top of master, so probably some of the changes will also need to be ported to `tools/include/linux/btf_ids.h`?
the fix in [1] is fixing flags in set8's pairs, but not the global flags
it looks like Viktor's fix should now also swap that as well? like in the change below on top of Viktor's changes (untested)
jirka
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index d01603ef6283..c44d57fec390 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -706,6 +706,8 @@ static int sets_patch(struct object *obj) * correctly translate everything. */ if (need_bswap) {
set8->flags = bswap_32(set8->flags);for (i = 0; i < cnt; i++) { set8->pairs[i].flags = bswap_32(set8->pairs[i].flags);
That should work. Here are a few tests I ran:
$ md5sum /tmp/kbuild-s390x/vmlinux.* eb658e51e089f3c5b2c8909a29dc9997 /tmp/kbuild-s390x/vmlinux.a # plain vmlinux before running resolv_btfids (all 0s) ea907cd46a1a73b8276b5f2a82af00ca /tmp/kbuild-s390x/vmlinux.before_resolv # x86_64 resolv_btfids on master without Viktor's patch 980a40c3a3ff563d1c2d1ebdd5071a23 /tmp/kbuild-s390x/vmlinux.resolv_native # x86_64 resolv_btfids on master with Viktor's patch b986d19e242719ebea41c578235da662 /tmp/kbuild-s390x/vmlinux.resolv_native_patch_viktor # x86_64 resolv_btfids on master with Viktor's patch and your suggested patch 4edd8752ff01129945bd442689b1927b /tmp/kbuild-s390x/vmlinux.resolv_native_patch_viktor_patched # s390x resolv_btfids run with qemu-s390x-static 4edd8752ff01129945bd442689b1927b /tmp/kbuild-s390x/vmlinux.resolv_s390x
and some hexdiff of those binaries:
# difference between master's native build and s390x build.... has byte swapping for set8 and others diff -ruN <(xxd /tmp/kbuild-s390x/vmlinux.resolv_s390x) <(xxd /tmp/kbuild-s390x/vmlinux.resolv_native) > diff_s390x_native.diff https://gist.github.com/chantra/c3d58637a08a6f7340953dc155bb18cc
# difference betwee Viktor's version and s390x build.... squinting my eyes I only see the global set8 is missing diff -ruN <(xxd /tmp/kbuild-s390x/vmlinux.resolv_s390x) <(xxd /tmp/kbuild-s390x/vmlinux.resolv_native_patch_viktor) > diff_s390x_native_viktor.diff https://gist.github.com/chantra/61cfff02b456ae72d3c0161ce1897097
Have a good weekend all!
Manu