On Fri, Mar 13, 2026 at 2:30 AM Hari Bathini <[email protected]> wrote:
>
>
>
> On 12/03/26 11:52 pm, Yonghong Song wrote:
> >
> >
> > On 3/12/26 1:01 AM, Hari Bathini wrote:
> >> On powerpc, immediate load instructions are sign extended. In case
> >> of unsigned types, arguments should be explicitly zero-extended by
> >> the caller. For kfunc call, this needs to be handled in the JIT code.
> >> In bpf_kfunc_call_test4(), that tests for sign-extension of signed
> >> argument types in kfunc calls, add some additional failure checks.
> >> And add bpf_kfunc_call_test5() to test zero-extension of unsigned
> >> argument types in kfunc calls.
> >>
> >> Signed-off-by: Hari Bathini <[email protected]>
> >
> > LGTM with a nit below.
> >
> > Acked-by: Yonghong Song <[email protected]>
>
> Thanks for the review, Yonghong.
>
> >
> >> ---
> >>
> >> Changes in v2:
> >> - Added asm version of the selftest for consistent testing across
> >>    different BPF ISA versions.
> >> - Added comments clearly stating the intent of the test cases.
> >> - Updated sign-extension selftest to have additional failure checks.
> >>
> >>
> >>   .../selftests/bpf/prog_tests/kfunc_call.c     |  2 +
> >>   .../selftests/bpf/progs/kfunc_call_test.c     | 98 +++++++++++++++++++
> >>   .../selftests/bpf/test_kmods/bpf_testmod.c    | 54 +++++++++-
> >>   .../bpf/test_kmods/bpf_testmod_kfunc.h        |  1 +
> >>   4 files changed, 154 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/
> >> tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> >> index f79c8e53cb3e..62f3fb79f5d1 100644
> >> --- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> >> +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> >> @@ -74,6 +74,8 @@ static struct kfunc_test_params kfunc_tests[] = {
> >>       TC_TEST(kfunc_call_test1, 12),
> >>       TC_TEST(kfunc_call_test2, 3),
> >>       TC_TEST(kfunc_call_test4, -1234),
> >> +    TC_TEST(kfunc_call_test5, 0),
> >> +    TC_TEST(kfunc_call_test5_asm, 0),
> >>       TC_TEST(kfunc_call_test_ref_btf_id, 0),
> >>       TC_TEST(kfunc_call_test_get_mem, 42),
> >>       SYSCALL_TEST(kfunc_syscall_test, 0),
> >> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/
> >> tools/testing/selftests/bpf/progs/kfunc_call_test.c
> >> index 8b86113a0126..5edc51564f71 100644
> >> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> >> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> >> @@ -2,8 +2,106 @@
> >>   /* Copyright (c) 2021 Facebook */
> >>   #include <vmlinux.h>
> >>   #include <bpf/bpf_helpers.h>
> >> +#include "bpf_misc.h"
> >>   #include "../test_kmods/bpf_testmod_kfunc.h"
> >> +SEC("tc")
> >> +int kfunc_call_test5(struct __sk_buff *skb)
> >> +{
> >> +    struct bpf_sock *sk = skb->sk;
> >> +    int ret;
> >> +    u32 val32;
> >> +    u16 val16;
> >> +    u8 val8;
> >> +
> >> +    if (!sk)
> >> +        return -1;
> >> +
> >> +    sk = bpf_sk_fullsock(sk);
> >> +    if (!sk)
> >> +        return -1;
> >> +
> >> +    /*
> >> +     * Test with constant values to verify zero-extension.
> >> +     * ISA-dependent BPF asm:
> >> +     *   With ALU32:    w1 = 0xFF; w2 = 0xFFFF; w3 = 0xFFFFffff
> >> +     *   Without ALU32: r1 = 0xFF; r2 = 0xFFFF; r3 = 0xFFFFffff
> >> +     * Both zero-extend to 64-bit before the kfunc call.
> >> +     */
> >> +    ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFffffULL);
> >
> > Can we just use 0xFFFFffff instead of 0xFFFFffffULL?
>
> Alexei, can you confirm if I need to respin with this change?

I prefer the explicit type when the constant is on the boundary.

I was about to apply it, but it fails on s390. See CI:

Error: #160 kfunc_call
Error: #160/13 kfunc_call/kfunc_call_test5_asm
Error: #160/13 kfunc_call/kfunc_call_test5_asm
verify_success:PASS:skel 0 nsec
verify_success:PASS:bpf_object__find_program_by_name 0 nsec
verify_success:PASS:kfunc_call_test5_asm 0 nsec
verify_success:FAIL:retval unexpected retval: actual 6 != expected 0
Test Results:
bpftool: PASS
test_progs: FAIL (returned 1)

It doesn't look to be endianness related (which is often
the case for breaking s390).
In this case it could be an s390 JIT issue?

Ilya ?

pw-bot: cr

Reply via email to