Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LoongArch: BPF: Sign-extend return values #4681

Closed

Conversation

kernel-patches-daemon-bpf-rc[bot]
Copy link

Pull request for series with
subject: LoongArch: BPF: Sign-extend return values
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 2c8b09a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 2c8b09a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 2c8b09a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: fc39fb5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: fc39fb5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: fc39fb5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 28eb75e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 28eb75e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

(1) Description of Problem:

When testing BPF JIT with the latest compiler toolchains on LoongArch,
there exist some strange failed test cases, dmesg shows something like
this:

  # dmesg -t | grep FAIL | head -1
  ... ret -3 != -3 (0xfffffffd != 0xfffffffd)FAIL ...

(2) Steps to Reproduce:

  # echo 1 > /proc/sys/net/core/bpf_jit_enable
  # modprobe test_bpf

(3) Additional Info:

There are no failed test cases compiled with the lower version of GCC
such as 13.3.0, while the problems only appear with higher version of
GCC such as 14.2.0.

This is because the problems were hidden by the lower version of GCC
due to there are redundant sign extension instructions generated by
compiler, but with optimization of higher version of GCC, the sign
extension instructions have been removed.

(4) Root Cause Analysis:

The LoongArch architecture does not expose sub-registers, and hold all
32-bit values in a sign-extended format. While BPF, on the other hand,
exposes sub-registers, and use zero-extension (similar to arm64/x86).

This has led to some subtle bugs, where a BPF JITted program has not
sign-extended the a0 register (return value in LoongArch land), passed
the return value up the kernel, for example:

  | int from_bpf(void);
  |
  | long foo(void)
  | {
  |    return from_bpf();
  | }

Here, a0 would be 0xffff_ffff, instead of the expected
0xffff_ffff_ffff_ffff.

Internally, the LoongArch JIT uses a5 as a dedicated register for BPF
return values. That is to say, the LoongArch BPF uses a5 for BPF return
values, which are zero-extended, whereas the LoongArch ABI uses a0 which
is sign-extended.

(5) Final Solution:

Keep a5 zero-extended, but explicitly sign-extend a0 (which is used
outside BPF land). Because libbpf currently defines the return value
of an ebpf program as a 32-bit unsigned integer, just use addi.w to
extend bit 31 into bits 63 through 32 of a5 to a0. This is similar
with commit 2f1b0d3 ("riscv, bpf: Sign-extend return values").

Fixes: 5dc6155 ("LoongArch: Add BPF JIT support")
Signed-off-by: Tiezhu Yang <[email protected]>
Acked-by: John Fastabend <[email protected]>
@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 9f16d5e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=910839
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=910839 irrelevant now. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant