[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] Revert "LoongArch: BPF: Sign extend kfunc call arguments"

[opsiff]

deepin inclusion
category: bugfix

It cause loongarch build failed. it need merge more depends, such as "LoongArch: Add more instruction opcodes and emit_* helpers" This reverts commit 83aa5d5.

Summary by Sourcery

Bug Fixes:

• Fix LoongArch build failures by dropping the new kfunc argument extension logic in the BPF JIT implementation.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Reverts the LoongArch BPF JIT kfunc-argument ABI extension logic to fix build issues by removing the helper that extended kfunc arguments and its use in kfunc call generation.

Updated class diagram for LoongArch BPF JIT kfunc handling after revert

classDiagram
    class JitCtx {
    }

    class BpfInsn {
    }

    class BtfFuncModel {
        +int nr_args
        +u8 arg_size[]
        +u8 arg_flags[]
    }

    class LoongArchBpfJit_PreRevert {
        +int build_insn(const BpfInsn *insn, JitCtx *ctx, bool extra_pass)
        +void emit_sext_32(JitCtx *ctx, int reg, bool dst)
        +void emit_abi_ext(JitCtx *ctx, int reg, u8 size, bool sign)
        +void move_addr(JitCtx *ctx, int rd, u64 addr)
    }

    class LoongArchBpfJit_PostRevert {
        +int build_insn(const BpfInsn *insn, JitCtx *ctx, bool extra_pass)
        +void emit_sext_32(JitCtx *ctx, int reg, bool dst)
        +void move_addr(JitCtx *ctx, int rd, u64 addr)
    }

    LoongArchBpfJit_PreRevert ..> BtfFuncModel : uses
    LoongArchBpfJit_PostRevert ..|> LoongArchBpfJit_PreRevert : logical_evolution

    LoongArchBpfJit_PreRevert ..> JitCtx : uses
    LoongArchBpfJit_PreRevert ..> BpfInsn : uses

    LoongArchBpfJit_PostRevert ..> JitCtx : uses
    LoongArchBpfJit_PostRevert ..> BpfInsn : uses

Loading

File-Level Changes

Change	Details	Files
Remove ABI-specific extension of BPF kfunc call arguments in the LoongArch BPF JIT.	Delete the emit_abi_ext helper that applied size- and sign-aware register extension according to ABI rules in the LoongArch BPF JIT header Remove the call site that, for BPF_PSEUDO_KFUNC_CALL instructions, queried the kfunc BTF model and applied emit_abi_ext to each kfunc argument register before issuing the call	arch/loongarch/net/bpf_jit.h arch/loongarch/net/bpf_jit.c

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Sorry @opsiff, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deepin-ci-robot]

deepin pr auto review

这段代码的 diff 展示了对 LoongArch 架构下 BPF JIT 编译器的修改。主要改动是删除了处理 BPF 辅助函数调用参数扩展的代码逻辑。

以下是对这段代码变更的审查意见，涵盖语法逻辑、代码质量、性能和安全方面：

1. 语法逻辑与代码质量

• 变更意图分析：

  • 原代码逻辑：在 build_insn 函数中，当遇到 BPF_PSEUDO_KFUNC_CALL（内核函数调用）时，代码会查找函数模型（btf_func_model），并根据参数的大小和符号属性，调用 emit_abi_ext 对参数寄存器进行符号扩展或零扩展，以符合 LoongArch ABI 的调用约定。
  • 变更后逻辑：这段逻辑被完全移除了。这意味着编译器不再在 JIT 生成阶段显式处理 KFunc 调用参数的扩展。

• 潜在风险：

  • ABI 合规性：LoongArch 的 ABI 通常规定传递给函数的参数需要满足特定的宽度要求（例如，小于寄存器宽度的类型需要正确扩展）。移除 emit_abi_ext 后，如果 BPF 程序传递给 KFunc 的参数未在 JIT 生成前（例如在 BPF 验证器阶段）被正确处理，可能会导致传递给内核函数的参数值错误（如高位垃圾数据），从而引发内核行为异常或崩溃。
  • 一致性：删除 emit_abi_ext 函数本身是合理的，因为它不再被使用。但在 build_insn 中删除调用逻辑的前提必须是：参数扩展的逻辑已经在其他地方（如 BPF 核心验证逻辑或通用 JIT 层）得到了保证。如果这是将特定架构的处理逻辑上移到了通用代码中，那么这是一个好的重构；如果仅仅是删除而没有替代方案，则是一个严重的 Bug。

2. 代码性能

• 性能影响：
  • 从性能角度看，移除这些 JIT 指令（extwb, extwh, addiw）减少了生成的指令数量，理论上会提高 BPF 程序的执行速度，减少了函数调用的开销。
  • 前提：这种性能提升必须是安全的。如果省略了必要的扩展指令，CPU 可能需要处理未定义的高位数据，或者在后续计算中产生错误结果，这种“性能提升”是以正确性为代价的，是不可接受的。

3. 代码安全

• 安全性审查：
  • 数据完整性：BPF 程序运行在内核态，参数传递错误可能导致内核函数接收到错误的指针或数值。最坏的情况下，这可能被利用来绕过安全检查或导致内核崩溃（DoS）。
  • 验证器依赖：此修改的安全性完全依赖于 BPF 验证器。如果验证器能够确保所有加载到寄存器中的值（无论是通过 BPF_LD 还是其他指令）都已经根据目标类型进行了正确的扩展（即 BPF 寄存器中的值在传递给 helper/kfunc 时已经是“干净”的），那么移除 JIT 中的扩展逻辑是安全的。
  • 建议：需要确认上游 BPF 子系统（如 kernel/bpf/verifier.c）是否有对应的提交，确保在 verifier 阶段完成了类型转换和扩展。如果仅仅是 LoongArch 后端删除了这段代码，而其他架构（如 x86, arm64）保留了类似逻辑，那么这很可能是一个引入错误的 Patch。

4. 改进建议

1. 确认上下文：

   • 如果这是为了修复 Bug 或重构，请确保相关的 Commit Message 或代码注释解释了为什么不再需要这些扩展指令。通常是因为 BPF 寄存器现在在 verifier 阶段就已经被标记为具有特定的类型和大小，且加载指令已经处理了扩展。
   • 如果没有明确的理由，建议回退此改动，除非能证明参数传递总是满足 ABI 要求。

2. 防御性编程：

   • 如果决定保留此改动，建议在 build_insn 的其他地方（例如通用的参数准备阶段）添加断言或注释，说明参数此时必须已经是正确扩展后的值，以便于后续维护者理解。

3. 代码清理：

   • 删除未使用的 emit_abi_ext 函数是好的做法（代码中已包含此操作），这减少了死代码，提高了可维护性。

总结

这段代码变更主要是删除了 KFunc 调用前的参数符号扩展逻辑。

• 如果这是配合 BPF 核心层（Verifier）的改动，即参数扩展由通用逻辑统一处理，那么这是一个正确且优化的改动，减少了冗余指令。
• 如果这是孤立的改动，没有其他地方保证参数的扩展，那么这是一个高危的改动，会导致违反 ABI 规约，引发内核错误。

结论：在缺乏上下文（如关联的 Verifier 改动）的情况下，此改动存在较大的逻辑风险。建议审查相关的 BPF Verifier 代码，确认参数扩展是否已在更早阶段完成。

[Copilot]

Pull request overview

This pull request reverts a previous commit that attempted to add sign extension for kfunc call arguments in the LoongArch BPF JIT implementation. The revert is necessary because the original commit depends on instruction opcodes (extwb and extwh) and corresponding emit helper functions that are not yet available in the current kernel version, causing build failures.

Changes:

• Removed the emit_abi_ext helper function from arch/loongarch/net/bpf_jit.h
• Removed kfunc argument extension logic from the BPF_JMP | BPF_CALL case in arch/loongarch/net/bpf_jit.c

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
arch/loongarch/net/bpf_jit.h	Removed emit_abi_ext function that used undefined LoongArch instructions
arch/loongarch/net/bpf_jit.c	Removed kfunc argument extension code block that called the now-removed emit_abi_ext function

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.