feat(ebpf): decouple JIT to rbpf and migrate eBPF user tests to apps#891
feat(ebpf): decouple JIT to rbpf and migrate eBPF user tests to apps#891CN-TangLin wants to merge 35 commits into
Conversation
There was a problem hiding this comment.
PR #891 Review: eBPF JIT 编译器框架 + RISC-V 64 后端
感谢你的贡献!这是一个相当大的 PR(~4000 行),为 StarryOS 添加了完整的 eBPF JIT 编译框架和 riscv64 后端。整体架构设计合理,两遍编译策略与 Linux 内核一致。但我在代码审查中发现了若干需要关注的问题,按严重程度分类如下:
🔴 需要修改的问题
-
emit_load_imm6464位立即数加载序列有 bug- 当
hi52 >> 32 != 0时,当前的 LUI+ADDI+SLLI 序列不正确:中间mid20只取了hi52 >> 32的低 20 位,但丢失了hi52[51:32]的高位部分。完整的 64 位立即数加载应使用 6 条指令序列(LUI+ADDI+SLLI+ADDI+SLLI+ADDI+SRLI),参考 Linux 内核的 RV64 实现。 insn_size中对 LD_IMM64 估算为 24 字节,但实际 emit_load_imm64 在大立即数情况下可能需要 28 字节,导致缓冲区溢出风险。
- 当
-
JitBuffer未实现Send/Sync,但被存储在BpfProg中JitBuffer包含裸指针*mut u8,Rust 编译器不会自动为其实现Send/Sync。BpfProg被BpfFdTable持有,而BpfFdTable在SpinNoIrq后面跨线程共享。需要为JitBuffer添加unsafe impl Send/Sync(如果确实线程安全的话),或加注释说明为什么不需要。
-
emit_jmp中BPF_EXIT处理不正确BPF_EXIT分支中if target == 0 {}是一个空 if 语句(看起来像未完成的逻辑),然后无条件加载 imm64=0 并执行 JALR 到地址 0,这会导致跳转到地址 0 而非 epilogue。- 正确做法应该是计算到 prologue 起始位置(即
offsets[0])的偏移量并跳转回去执行 epilogue。但这需要知道编译后的 epilogue 在哪里。建议参考 Linux kernel 中 riscv64 BPF JIT 的 EXIT 处理方式——把 EXIT 统一跳转到一个固定的 epilogue 位置。
-
除零保护中分支修补的偏移计算
BPF_DIV/BPF_MOD中rv_b(skip_off as u32 * 2, ...)将偏移乘以 2——但 RV B-type imm 是字节为单位的,而skip_off已经是字节偏移差。这会导致偏移翻倍。
🟡 建议改进
rv_r+ XOR opcode 切换方式可读性差 — 建议直接定义rv_rw函数emit_call参数寄存器 shuffle 需要验证 — BPF r1-r5 对应 A1-A5 的移动是否与 helper 签名匹配ebpf_jit子模块中super::super::路径不清晰 — 建议在 mod.rs 中 re-exportperf_event.rs缺少模块声明 — 被ebpf.rs通过crate::perf_event引用但未在lib.rs/mod.rs中声明
📝 其他观察
sys_bpf和sys_perf_event_open在syscall/mod.rs中被路由到sys_dummy_fd,JIT 实现目前不会被实际调用- Stub 后端设计合理,后续可逐步实现
JitBuffer::finalize()中 aarch64 的 dcache clean + icache flush 实现正确
总结
PR 的整体架构设计是好的,但 JIT 后端中存在几个关键的代码生成 bug(特别是 emit_load_imm64、BPF_EXIT 处理和分支偏移计算),这些可能导致生成的机器码不正确。建议在合入前重点修复这些问题,并添加针对 JIT 编译结果的单元测试来验证生成的 RISC-V 指令序列。
Powered by glm-5.1
| emit_addi(buf, rd, rd, ((mid20 >> 12) & 0xfff) as i32); | ||
| emit_slli(buf, rd, rd, 8); | ||
| } | ||
| } |
There was a problem hiding this comment.
这段 64 位立即数加载序列在大立即数情况下不正确。当 hi52 >> 32 != 0 时:
- 最后一步
emit_slli(rd, rd, 8)只左移 8 位,但根据之前的 SLLI 12 + ADDI 模式,这里应该还需要移位 12 位来给最后的 ADDI 留出低 12 位空间 - 整个序列缺少最终的 SRLI 12 来消除多余的移位
建议参考 RISC-V 手册中的标准 64 位立即数加载序列(6 条指令):
LUI rd, upper[31:12]
ADDI rd, rd, lower[11:0]
SLLI rd, rd, 12
ADDI rd, rd, next[11:0]
SLLI rd, rd, 12
ADDI rd, rd, final[11:0]
同时,insn_size 中 LD_IMM64 估算为 24 字节可能不够——大立即数情况下 emit_load_imm64 会 emit 更多指令,需要同步更新。
| BPF_LSH => { | ||
| if use_imm { | ||
| let shamt = if is_64 { | ||
| (insn.imm as u32) & 63 |
There was a problem hiding this comment.
这里分支偏移修补中 skip_off * 2 看起来有误。RV B-type 指令的 immediate 是以字节为单位的,而 skip_off 已经是 after - skip(字节偏移差值)。乘以 2 会导致偏移量翻倍。
建议验证:beq 跳过 1 条 divu 指令 = 4 字节偏移。此时 skip_off = after - skip = 4(字节数),B-type imm 应为 4 而非 8。
| emit_jalr(buf, RV_ZERO, RV_ZERO, branch_off); | ||
| } | ||
| BPF_JGT => { | ||
| emit_bgeu(buf, dst, src, 8); |
There was a problem hiding this comment.
这段 BPF_EXIT 处理有 bug。if target == 0 {} 是一个空的 if 语句(未完成的逻辑),然后无条件加载 imm64=0 并执行 JALR 到地址 0——这会导致跳转到地址 0(空指针解引用),而非 epilogue。
正确做法:
- 在编译开始时记录 prologue 后的位置
- 所有 BPF_EXIT 跳转到一个固定的 epilogue 位置(在 buffer 末尾 emit 一次 epilogue)
参考 Linux kernel arch/riscv/net/bpf_jit_comp64.c 中 BPF_JMP_EXIT 的处理。
| pos: usize, | ||
| } | ||
|
|
||
| impl JitBuffer { |
There was a problem hiding this comment.
JitBuffer 包含 *mut u8 裸指针,Rust 不会自动为其实现 Send 和 Sync。但 JitBuffer 被存储在 BpfProg 中,而 BpfProg 在 BpfFdTable 中通过 SpinNoIrq 全局共享。
如果 JitBuffer 确实是线程安全的(分配的内存在 Drop 时释放,没有共享的内部可变性),建议添加:
// SAFETY: JitBuffer owns its memory allocation and has no shared interior mutability.
// The raw pointer is only accessed through &mut self methods, and Drop deallocates it.
unsafe impl Send for JitBuffer {}
unsafe impl Sync for JitBuffer {}| buf.emit_u32(rv_i(imm as u32, rs1, 0, rd, 0x13)); | ||
| } | ||
|
|
||
| fn emit_addiw(buf: &mut JitBuffer, rd: u32, rs1: u32, imm: i32) { |
There was a problem hiding this comment.
这种用 XOR 来切换 opcode 的方式可读性较差。rv_r(...) | (0x3b ^ 0x33) 等价于把 opcode 从 0x33(OP)改为 0x3b(OP-32)。
虽然数学上正确,但建议直接在编码函数中传入正确的 opcode,或定义专用函数:
fn rv_rw(funct7: u32, rs2: u32, rs1: u32, funct3: u32, rd: u32) -> u32 {
(funct7 << 25) | (rs2 << 20) | (rs1 << 15) | (funct3 << 12) | (rd << 7) | 0x3b
}这样代码意图更清晰,也避免了 XOR 操作的隐式依赖。
| @@ -0,0 +1,430 @@ | |||
| //! Performance event ring buffer infrastructure for StarryOS. | |||
There was a problem hiding this comment.
perf_event.rs 通过 crate::perf_event::... 被 ebpf.rs 引用,但我没有在 lib.rs 或任何 mod.rs 中找到 mod perf_event; 的声明。
请确认这个模块是否已通过其他机制(如 feature gate、条件编译等)注册到 crate 模块树中。如果缺失声明,编译时会报错。如果已有声明,请忽略此评论。
There was a problem hiding this comment.
PR #891 Review: eBPF JIT 编译器框架 + RISC-V 64 后端
感谢贡献!本 PR 为 StarryOS 添加了 eBPF JIT 编译框架和 riscv64 后端,整体架构设计合理(两遍编译策略与 Linux 内核一致),代码组织清晰。但我在逐行审查中发现若干阻塞性问题,主要集中在 JIT 后端的代码生成正确性上。
🔴 阻塞性问题
-
emit_call寄存器 shuffle 顺序完全错误- 当前 shuffle:
T2←A5, A5←A4, A4←A3, A3←A2, A2←A1, A1←A0 - 结果:A0=BPF r0(未变), A1=BPF r0(重复), A2=BPF r1, A3=BPF r2, A4=BPF r3, A5=BPF r4, T2=BPF r5
- 正确应为:A0←r1(A1), A1←r2(A2), A2←r3(A3), A3←r4(A4), A4←r5(A5)
- BPF r5 存在 A5,被保存到 T2 后从未移入 A4,导致第 5 个 helper 参数丢失;同时 A0 和 A1 都成了 r0,r1/r2 被错位到 A2/A3
- 这是 helper 调用必然产生错误结果 的 bug
- 当前 shuffle:
-
emit_load_imm6464 位立即数加载序列有 buglo12 = (val as i16) as i32取低 16 位符号扩展,但 RV ADDI 立即数只有 12 位,此处应取低 12 位- 大立即数分支(
hi52 >> 32 != 0)中,mid20仅取hi52[51:32]的低 20 位,但hi52[63:52]完全丢失 - 最后一步
slli 8是移入了一个不完整的偏移,缺少对应的 SRLI 来恢复 - 建议参考 Linux 内核
arch/riscv/net/bpf_jit_comp64.c的标准 6 条指令序列(LUI+ADDI+SLLI+ADDI+SLLI+ADDI),或使用 RV64 的构建 64 位常量标准方法 insn_size中 LD_IMM64 估算为 24 字节,但emit_load_imm64在大立即数情况下实际 emit 6 条指令(24 字节),与估算一致——但分解逻辑不正确意味着生成的指令值是错的
-
除零保护中分支偏移
skip_off * 2错误rv_b(skip_off as u32 * 2, RV_ZERO, src, 0)将偏移翻倍skip_off = after - skip已经是字节偏移差(BEQ 跳过 DIVU = 8 字节),rv_b()的 imm 参数也是以字节为单位- 传入
8 * 2 = 16会导致 BEQ 跳转到错误位置(跳过 2 条额外指令而非 1 条) - 应直接使用
rv_b(skip_off as u32, RV_ZERO, src, 0)
-
与 PR #888 存在完整重叠
🟡 建议改进
-
emit_jmp中 BPF_EXIT 的死代码:if target == 0 {}是空 if 语句,后面加载 0 并 JALR 到地址 0 会崩溃。虽然 mod.rs 中已直接调用emit_epilogue处理 BPF_EXIT(此处是死代码),但建议清理或改为unreachable!() -
BPF_LSH64位寄存器源模式下 ANDI 在 SLL 之后:emit_sll(buf, dst, dst, src); emit_andi(buf, src, src, 63)中 ANDI 在 SLL 之后,无实际效果。RV64 的 SLL 硬件已只取低 6 位,ANDI 应去掉或移到 SLL 之前 -
JitBuffer 未实现 Send/Sync:
JitBuffer包含*mut u8,不自动实现 Send。被BpfProg持有,而BpfProg在SpinNoIrq<BpfFdTable>后面。如果 SpinNoIrq 要求 T: Send,这可能导致编译问题。建议显式添加unsafe impl Send for JitBuffer {}(在 SpinNoIrq 保护下是安全的) -
emit_jalr(buf, RV_ZERO, RV_ZERO, jal_off)用于无条件跳转:JALR 的 rs1=RV_ZERO 表示跳转到地址 0 + jal_off,这是一个绝对地址而非 PC 相对跳转。确认这是预期行为——因为 offset 已是绝对 buffer 偏移而非 PC 相对偏移
📝 其他观察
perf_event.rs被ebpf.rs通过crate::perf_event引用,但未在lib.rs/mod.rs中看到mod perf_event声明——需要确认是否有 feature gate 或其他机制使其可见sys_bpf和sys_perf_event_open在syscall/mod.rs中仍路由到sys_dummy_fd,JIT 代码当前不会被实际调用- Prologue/Epilogue 设计合理:callee-saved 保存/恢复、BPF 栈分配、帧指针设置均正确
JitBuffer::finalize()的 aarch64 dcache clean + icache flush 实现正确- Rust 安全性方面:helper 函数通过
core::mem::transmute将函数指针转为可调用对象,需要确保调用约定匹配
验证状态
- ✅
cargo fmt --check通过 ⚠️ 未提供 JIT 编译结果的单元测试来验证生成的 RISC-V 指令序列⚠️ 无实际 QEMU 运行验证
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器 + 验证器 + map 类型 + helper + perf_event)。本 PR 的第一个 commit 与其完全相同。关系:partial-overlap,#888 是 #891 的前置依赖
- 未发现其他 eBPF JIT 相关的 open PR
总结
PR 的整体架构方向正确,但 JIT 后端中存在多个严重的代码生成 bug(helper 调用寄存器 shuffle 全错、64 位立即数加载分解不正确、分支偏移翻倍),这些问题会导致生成的机器码产生错误结果或崩溃。建议重点修复上述 1-3 号问题,添加针对 JIT 编译输出的单元测试,并在 rebase 到 #888 后重新提交。
Powered by glm-5.1
| emit_sb(buf, RV_T2, RV_T1, 0); | ||
| } | ||
| BPF_H => { | ||
| emit_load_imm32(buf, RV_T2, val as i32); |
There was a problem hiding this comment.
寄存器 shuffle 顺序完全错误。 eBPF helper 调用约定要求 r1-r5 作为参数,映射到 RV A1-A5。调用 Rust 函数时需要将参数移到 RV 调用约定的 A0-A4。
当前 shuffle 结果:
- A0 = 旧 A0 (BPF r0/返回值)
- A1 = 旧 A0 (BPF r0,重复!)
- A2 = 旧 A1 (BPF r1)
- A3 = 旧 A2 (BPF r2)
- A4 = 旧 A3 (BPF r3)
- A5 = 旧 A4 (BPF r4)
- T2 = 旧 A5 (BPF r5,丢失!)
正确应该是:
emit_mv(buf, RV_T2, RV_A5); // save r5
emit_mv(buf, RV_A4, RV_A5); // wait, we just clobbered A5...
实际上需要从高到低移动避免覆盖:
// save r5 to temp first
emit_mv(buf, RV_T2, RV_A5); // T2 = r5
emit_mv(buf, RV_A5, RV_A4); // A5 = r4 (not needed but harmless)
emit_mv(buf, RV_A4, RV_T2); // A4 = r5 ✓
emit_mv(buf, RV_T2, RV_A3); // T2 = r3
emit_mv(buf, RV_A3, RV_A2); // A3 = r2
emit_mv(buf, RV_A2, RV_A1); // A2 = r1
emit_mv(buf, RV_A1, RV_A0); // A1 = r0 (context)
// Now: A0=context(r0), A1=r0, A2=r1, A3=r2, A4=r5等等,让我重新理清:BPF helper 的第一个参数是 r1(context),在 RV 中应放在 A0。所以:
- A0 ← r1(=A1)
- A1 ← r2(=A2)
- A2 ← r3(=A3)
- A3 ← r4(=A4)
- A4 ← r5(=A5)
正确的 shuffle(从高到低避免覆盖):
emit_mv(buf, RV_T2, RV_A5); // T2 ← r5
emit_mv(buf, RV_A4, RV_A4); // A4 ← r4 (原地,但下面要被覆盖)
// 实际需要两个 temp:
emit_mv(buf, RV_T1, RV_A4); // T1 ← r4
emit_mv(buf, RV_A4, RV_T2); // A4 ← r5 ✓
emit_mv(buf, RV_T2, RV_A3); // T2 ← r3
emit_mv(buf, RV_A3, RV_A2); // A3 ← r2 ✓
emit_mv(buf, RV_A2, RV_A1); // A2 ← r1 ✓
emit_mv(buf, RV_A1, RV_A3); // wait...最简单的方式:直接对应移动,r1→A0, r2→A1 等。由于 BPF r1-r5 = RV A1-A5,需要 A0←A1, A1←A2, A2←A3, A3←A4, A4←A5。
从高到低避免覆盖:
emit_mv(buf, RV_A4, RV_A5); // A4 ← r5
emit_mv(buf, RV_A3, RV_A4); // A3 ← r4
emit_mv(buf, RV_A2, RV_A3); // A2 ← r3
emit_mv(buf, RV_A1, RV_A2); // A1 ← r2
emit_mv(buf, RV_A0, RV_A1); // A0 ← r1这个顺序有问题,因为 A5 先被移到 A4,然后 A4(现在是 r5)又被移到 A3。正确应该是从 A5 开始向前移,但由于 A5 的值需要到 A4,而 A4 需要到 A3 等,用 2 个 temp 寄存器即可:
// 保存最后两个需要保护的值
emit_mv(buf, RV_T1, RV_A4); // T1 = r4
emit_mv(buf, RV_T2, RV_A5); // T2 = r5
// 现在可以从前向后移动
emit_mv(buf, RV_A0, RV_A1); // A0 = r1 ✓
emit_mv(buf, RV_A1, RV_A2); // A1 = r2 ✓
emit_mv(buf, RV_A2, RV_A3); // A2 = r3 ✓
emit_mv(buf, RV_A3, RV_T1); // A3 = r4 ✓
emit_mv(buf, RV_A4, RV_T2); // A4 = r5 ✓| emit_addi(buf, rd, rd, ((mid20 >> 12) & 0xfff) as i32); | ||
| emit_slli(buf, rd, rd, 8); | ||
| } | ||
| } |
There was a problem hiding this comment.
64 位立即数加载序列在大立即数情况下不正确。
问题 1:lo12 = (val as i16) as i32 取的是 val 的低 16 位再符号扩展到 32 位,但 RV ADDI 的立即数只有 12 位。正确应为:let lo12 = (val as i32) << 20 >> 20;(取低 12 位符号扩展)或等价的位操作。
问题 2:大立即数分支中,mid20 = (hi52 >> 32) & 0xfffff 只取了 hi52 的 bits[51:32](20 位),但 hi52 最高可达 bits[63:0](52 位),bits[63:52] 完全丢失。
问题 3:最后的 emit_slli(rd, rd, 8) 移入 8 位后无法恢复这些位移前的值。
建议参考 Linux 内核 RV64 BPF JIT 的 emit_imm 实现,使用标准 6 条指令序列构建任意 64 位常量。
| BPF_LSH => { | ||
| if use_imm { | ||
| let shamt = if is_64 { | ||
| (insn.imm as u32) & 63 |
There was a problem hiding this comment.
分支偏移计算 skip_off * 2 有误。
skip_off = after - skip = 8(BEQ 占 4 字节 + DIVU 占 4 字节 = 8 字节偏移)。
rv_b(imm, ...) 的 imm 参数是以字节为单位的分支偏移量。所以应该传入 skip_off = 8(跳过 8 字节到 DIVU 之后)。
当前 skip_off * 2 = 16,会导致 BEQ 在 src==0 时跳过 16 字节(4 条指令而非 2 条),可能跳过后续的有效代码。
修复:将 rv_b(skip_off as u32 * 2, ...) 改为 rv_b(skip_off as u32, ...)。BPF_MOD 中也有同样的问题。
| emit_jalr(buf, RV_ZERO, RV_ZERO, branch_off); | ||
| } | ||
| BPF_JNE => { | ||
| emit_beq(buf, dst, src, 8); |
There was a problem hiding this comment.
BPF_EXIT 死代码存在崩溃风险。 if target == 0 {} 是空 if 语句(未完成的逻辑),然后 emit_load_imm64(buf, RV_T6, 0); emit_jalr(buf, RV_ZERO, RV_T6, 0) 会跳转到地址 0(空指针解引用/内存违规)。
虽然当前 mod.rs 的 compile() 中 BPF_EXIT 是直接调用 emit_epilogue() 而非走 emit_jmp(),所以这段代码目前是死代码。但建议:
- 如果确认不会走到这里,改为
unreachable!()或 panic - 如果未来可能走到,实现正确的 epilogue 跳转(记录 epilogue 位置,所有 EXIT 跳转到固定 epilogue)
| _ => {} | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
emit_mv(buf, RV_A0, RV_A0) 是一个 no-op(自己移动到自己),可以删除。
There was a problem hiding this comment.
PR #891 重新审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在第三个 commit 中修复了之前 review 提出的多个问题(rv_rw 辅助函数、Send/Sync、模块重组、BPF_EXIT 死代码清理、除零分支偏移 *2 移除)。但当前代码仍存在 2 个阻塞性 bug 和 1 个结构性问题。
🔴 阻塞性问题
1. emit_call 寄存器 shuffle 顺序完全错误
BPF helper 函数签名:fn(a1=r1, a2=r2, a3=r3, a4=r4, a5=r5) -> r0
RV 调用约定:arg1→A0, arg2→A1, arg3→A2, arg4→A3, arg5→A4
正确 shuffle:A0←r1(A1), A1←r2(A2), A2←r3(A3), A3←r4(A4), A4←r5(A5)
当前 shuffle 结果:A0=r0, A1=r0, A2=r1, A3=r2, A4=r3, T2=r5(丢失)
- A0 和 A1 都成了 r0(r0 是返回值寄存器,不是参数)
- r5 被保存到 T2 后从未移入 A4,第 5 个 helper 参数永远丢失
- 所有参数错位,helper 调用必然产生错误结果
建议的正确实现:
emit_mv(buf, RV_T2, RV_A5); // save r5
emit_mv(buf, RV_A4, RV_A4); // no, need reverse order
// 正确顺序:从高到低移动避免覆盖
emit_mv(buf, RV_T2, RV_A5); // save r5 -> T2
emit_mv(buf, RV_A5, RV_A4); // not needed for helper
// ...
// 简化:
emit_mv(buf, RV_T2, RV_A5); // T2 = r5
emit_mv(buf, RV_A4, RV_A3); // A4 = r3 (临时)
emit_mv(buf, RV_A3, RV_A2); // A3 = r2
emit_mv(buf, RV_A2, RV_A1); // A2 = r1
emit_mv(buf, RV_A1, RV_A0); // A1 = ??? 应为 r0 但 r0 不变
// 实际上 BPF helper 的 A0 就是返回值,不需 shuffle
// 正确:A0←A1, A1←A2, A2←A3, A3←A4, A4←T2(saved A5)
emit_mv(buf, RV_T2, RV_A5);
emit_mv(buf, RV_A5, RV_A4);
emit_mv(buf, RV_A4, RV_A3);
emit_mv(buf, RV_A3, RV_A2);
emit_mv(buf, RV_A2, RV_A1);
emit_mv(buf, RV_A1, RV_A0); // 这行是错的等等,让我重新分析。BPF 的 r0 映射到 RV A0。Helper 返回值也在 A0(=r0)。所以 shuffle 不应动 A0。正确做法:
A0 保留 (r0, 也是返回值)
A1 ← r1 (已是 A1,不动)
A2 ← r2 (已是 A2,不动)
...
等等——BPF 寄存器映射已经是 r0=A0, r1=A1, ..., r5=A5!所以参数已经在正确位置了!不需要 shuffle!除非调用约定要求 caller-saved 保存。
不对,仔细看 BPF helper 调用约定:arg1 到 arg5 分别存在 r1-r5(即 A1-A5),返回值存入 r0(即 A0)。RV 调用约定中 arg1-arg5 就是 A0-A4。
所以实际需要的映射是:
- RV A0 ← BPF r1 (目前在 RV A1)
- RV A1 ← BPF r2 (目前在 RV A2)
- RV A2 ← BPF r3 (目前在 RV A3)
- RV A3 ← BPF r4 (目前在 RV A4)
- RV A4 ← BPF r5 (目前在 RV A5)
当前代码的 shuffle 完全错误。
2. emit_load_imm64 的 lo12 计算有 bug
lo12 = (val as i16) as i32 取低 16 位并符号扩展。但 ADDI 立即数只有 12 位有符号。
当 val 的 bits[15:12] 非零时,lo12 值不正确,导致 hi52 计算也跟着错。
例如 val=0x1000:lo12=4096(超出 ADDI 范围),hi52=0。LUI(rd,0); ADDI(rd,rd,4096) —— ADDI 溢出。
应改为:let lo12 = ((val as i64) << 52 >> 52) as i32;(符号扩展低 12 位)
3. 与 PR #888 存在完整重叠
PR #891 的第一个 commit 70fa1bedf 与 PR #888(同作者 CN-TangLin)的内容完全相同。本 PR 应仅在 #888 基础上添加 JIT 代码。建议先合并 #888,然后本 PR rebase 到 dev 分支。
🟡 建议改进
-
BPF_LSH64 位寄存器源模式:emit_sll(buf, dst, dst, src); emit_andi(buf, src, src, 63);中 ANDI 在 SLL 之后是死代码。RV64 硬件 SLL 只取 src 低 6 位,ANDI 应去掉(或 32 位模式中已在 T2 上正确处理)。 -
insn_size中 LD_IMM64 估算为 28 字节——emit_load_imm64在大立即数分支(hi52 >> 32 != 0)emit 7 条指令(LUI+ADDI+SLLI+ADDI+SLLI+ADDI+SLLI = 28 字节),加上可能的最终 ADDI(lo12) 是 32 字节,超出 insn_size 估算。建议仔细核对最坏情况。
📝 其他观察
- Prologue/Epilogue 设计正确
JitBuffer的Send/Sync和Debug已正确实现rv_rw辅助函数替代了 XOR opcode 切换,可读性好- 模块重组到
ebpf/子目录结构清晰 - 当前
sys_bpf路由到sys_dummy_fd,JIT 代码不会被实际调用 - 缺少 JIT 编译结果的单元测试
Duplicate/Overlap 分析
- PR #888(同作者):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 的第一个 commit 与其完全相同。关系:partial-overlap,#888 是 #891 的前置依赖。建议先合并 #888。
- 未发现其他 eBPF JIT 相关的 open PR
Powered by glm-5.1
| emit_mv(buf, RV_A2, RV_A1); | ||
| emit_mv(buf, RV_A1, RV_A0); | ||
|
|
||
| emit_load_imm64(buf, RV_T1, helper_fn as u64); |
There was a problem hiding this comment.
🔴 阻塞性 bug:寄存器 shuffle 顺序完全错误
BPF helper 函数调用约定:arg1→r1, arg2→r2, arg3→r3, arg4→r4, arg5→r5,返回值→r0。
RISC-V 调用约定:arg1→A0, arg2→A1, arg3→A2, arg4→A3, arg5→A4。
寄存器映射:r0=A0, r1=A1, r2=A2, r3=A3, r4=A4, r5=A5。
所以需要的 shuffle 是:
- RV A0 ← BPF r1(当前在 A1)
- RV A1 ← BPF r2(当前在 A2)
- RV A2 ← BPF r3(当前在 A3)
- RV A3 ← BPF r4(当前在 A4)
- RV A4 ← BPF r5(当前在 A5)
当前代码的 shuffle 结果:
- T2 ← A5 (save r5)
- A5 ← A4 → A5 = r4
- A4 ← A3 → A4 = r3
- A3 ← A2 → A3 = r2
- A2 ← A1 → A2 = r1
- A1 ← A0 → A1 = r0
最终:A0=r0(错误), A1=r0(重复), A2=r1, A3=r2, A4=r3, T2=r5(丢失)
正确实现应从高到低移动以避免覆盖:
emit_mv(buf, RV_A4, RV_A5); // A4 ← r5
emit_mv(buf, RV_A3, RV_A4); // A3 ← r4 (此时 A4 已被覆盖为 r5)不对,这样也不行。需要临时寄存器。建议使用 T2 保存 A5(r5),然后从低到高或用两个临时寄存器:
// 保存高编号寄存器到临时
emit_mv(buf, RV_T2, RV_A5); // T2 = r5
emit_mv(buf, RV_T6, RV_A4); // T6 = r4
// 从低到高移动
emit_mv(buf, RV_A0, RV_A1); // A0 = r1
emit_mv(buf, RV_A1, RV_A2); // A1 = r2
emit_mv(buf, RV_A2, RV_A3); // A2 = r3
emit_mv(buf, RV_A3, RV_T6); // A3 = r4
emit_mv(buf, RV_A4, RV_T2); // A4 = r5这是 所有 helper 调用必然产生错误结果 的严重 bug。
| fn emit_load_imm64(buf: &mut JitBuffer, rd: u32, val: u64) { | ||
| let lo12 = (val as i16) as i32; | ||
| let hi52 = ((val as i64) - lo12 as i64) as u64; | ||
| let hi20 = (hi52 >> 12) as u32; |
There was a problem hiding this comment.
🔴 阻塞性 bug:lo12 的计算方式不正确
lo12 = (val as i16) as i32 取低 16 位并从 bit 15 符号扩展。
但 RV ADDI 的立即数只有 12 位有符号(-2048 到 2047)。当 val 的 bits[15:12] 非全零或非全一时,lo12 值错误,hi52 计算也跟着错。
举例:val = 0x1000
lo12 = (0x1000i16) as i32 = 4096(超出 ADDI 12 位范围)hi52 = val - 4096 = 0- 生成:LUI(rd, 0); ADDI(rd, rd, 4096) ← ADDI 溢出,高 20 位被截断
正确应改为:
let lo12 = ((val as i64) << 52 >> 52) as i32; // 符号扩展低 12 位或者等价地:
let lo12 = ((val & 0xfff) as u16) as i16 as i32; // 取低 12 位符号扩展|
@CN-TangLin 这个PR我建议可以尝试提交到https://github.com/qmonnet/rbpf 上游仓库而不是在Starry中单独实现。 |
|
感谢 @Godones 的建议!这确实是一个更好的方向。我计划将 riscv64 JIT 后端贡献到 rbpf 上游仓库,具体方案如下:
当前 StarryOS PR 中的 eBPF 子系统(verifier、map、perf_event、kprobe hook 等)仍然保留在 StarryOS 中,只是 JIT 编译器部分改为依赖上游 rbpf。 |
d8e3ae0 to
a0e3754
Compare
There was a problem hiding this comment.
PR #891 第四轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前几轮 review 基础上持续修复。本次审查聚焦于当前 HEAD(a0e3754c)的代码正确性。
🔴 阻塞性问题
1. emit_call 寄存器 shuffle 仍然完全错误
BPF helper 调用约定:helper(r1, r2, r3, r4, r5) → r0
RISC-V 调用约定:func(a0, a1, a2, a3, a4) → a0
当前 BPF→RV 映射:r0→A0, r1→A1, r2→A2, r3→A3, r4→A4, r5→A5
Helper 调用需要的映射:
- RV A0 ← BPF r1(当前在 RV A1)
- RV A1 ← BPF r2(当前在 RV A2)
- RV A2 ← BPF r3(当前在 RV A3)
- RV A3 ← BPF r4(当前在 RV A4)
- RV A4 ← BPF r5(当前在 RV A5)
当前 shuffle 结果:
T2 ← A5 // T2 = r5(保存但从未移入 A4)
A5 ← A4 // A5 = r4
A4 ← A3 // A4 = r3
A3 ← A2 // A3 = r2
A2 ← A1 // A2 = r1
A1 ← A0 // A1 = r0(不应是参数)
结果:A0=r0, A1=r0, A2=r1, A3=r2, A4=r3, T2=r5(丢失)
所有 5 个 helper 参数错位,r5 完全丢失。这是必然产生错误结果的 bug。
正确实现(使用栈临时保存,最简单可靠):
emit_addi(buf, RV_SP, RV_SP, -40);
emit_sd(buf, RV_A1, RV_SP, 0);
emit_sd(buf, RV_A2, RV_SP, 8);
emit_sd(buf, RV_A3, RV_SP, 16);
emit_sd(buf, RV_A4, RV_SP, 24);
emit_sd(buf, RV_A5, RV_SP, 32);
emit_ld(buf, RV_A0, RV_SP, 0); // A0 = r1
emit_ld(buf, RV_A1, RV_SP, 8); // A1 = r2
emit_ld(buf, RV_A2, RV_SP, 16); // A2 = r3
emit_ld(buf, RV_A3, RV_SP, 24); // A3 = r4
emit_ld(buf, RV_A4, RV_SP, 32); // A4 = r5
emit_addi(buf, RV_SP, RV_SP, 40);2. emit_load_imm64 的 lo12 计算仍有 bug
当前代码:let lo12 = (val as i16) as i32;
这取 val 的低 16 位并符号扩展,但 ADDI 立即数只有 12 位有符号(-2048~2047)。
当 val 的 bits[15:12] 非零时(如 val=0x1000,lo12=4096),lo12 超出 12 位范围,rv_i(imm << 20, ...) 中 imm 的高位会污染 rs1/funct3/rd 字段,产生错误的指令编码。
应改为只符号扩展低 12 位:
let lo12 = ((val as i64) << 52 >> 52) as i32; // sign-extend bits[11:0]3. BPF_DIV 除零时未将 dst 清零
当除数为 0 时,当前代码跳过 DIVU 指令,dst 保持原值。但 Linux eBPF 规范要求 DIV by 0 返回 0,MOD by 0 保持 dst 不变。MOD 的处理是正确的,但 DIV 的处理不对。
修复:在 BEQ 跳转后添加一条 emit_addi(buf, dst, RV_ZERO, 0) 将 dst 清零:
BEQ src, x0, +12 // if src==0, skip to zero
DIVU dst, dst, src
JALR x0, x0, +8 // skip zeroing
ADDI dst, x0, 0 // dst = 0
🟡 建议改进
4. BPF_LSH/BPF_RSH/BPF_ARSH 64 位寄存器源模式中的死代码
emit_sll(buf, dst, dst, src); emit_andi(buf, src, src, 63); — ANDI 在 SLL 之后执行,对移位结果无影响。RV64 硬件 SLL 已经只取 src 低 6 位。建议移除 ANDI 或将其移到 SLL 之前(虽然功能上不需要)。
5. emit_jmp 中 BPF_EXIT 处理是死代码
编译器在 JitCompiler::compile() 中直接处理 BPF_EXIT(调用 emit_epilogue),emit_jmp 中的 BPF_EXIT 分支永远不会执行。但该分支中的 emit_jalr(buf, RV_ZERO, RV_ZERO, -(off as i32)) 实际上是跳转到负地址的 bug。建议改为 unreachable!() 或删除。
📝 其他观察
- Prologue/Epilogue 的 callee-saved 保存/恢复、BPF 栈帧分配、帧指针设置均正确
JitBuffer的Send/Sync和Debug已正确实现rv_rw辅助函数替代了之前的 XOR opcode 切换,可读性好JitBuffer::finalize()的架构相关缓存刷新(aarch64 dcache clean + icache flush)实现正确- 除零分支偏移的
*2问题已修复 ✅ - 模块从
ebpf_jit/移到ebpf/ebpf_jit/,super::super::路径已改为 re-export ✅
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 的前三个 commit 包含了 #888 的全部内容。
- 未发现其他 eBPF JIT 相关的 open PR
验证状态
- ✅
cargo fmt --check通过(PR 声明) ⚠️ 缺少 JIT 编译结果的单元测试来验证生成的 RISC-V 指令序列⚠️ 无实际 QEMU 运行验证(当前 sys_bpf 在 syscall 中已路由到 eBPF 模块)- ❌ helper 调用寄存器 shuffle 错误意味着任何使用 helper 的 eBPF 程序在 JIT 模式下都会产生错误结果
Powered by glm-5.1
| } else if (insn.imm as i32) >= -2048 && (insn.imm as i32) < 2048 { | ||
| 8 | ||
| } else { | ||
| 28 |
There was a problem hiding this comment.
🔴 阻塞性 bug:寄存器 shuffle 顺序完全错误
BPF helper 需要:A0←r1(A1), A1←r2(A2), A2←r3(A3), A3←r4(A4), A4←r5(A5)
当前 shuffle 结果:A0=r0, A1=r0, A2=r1, A3=r2, A4=r3, T2=r5(丢失)
所有 5 个参数错位 + r5 完全丢失。这是 helper 调用必然产生错误结果的 bug。
建议使用栈保存+重载的方式(最简单可靠,参见 review body 中的示例代码),或者使用两个 temp 寄存器乒乓式交替移动。
| } | ||
|
|
||
| fn emit_beq(buf: &mut JitBuffer, rs1: u32, rs2: u32, off: i32) { | ||
| buf.emit_u32(rv_b(off as u32, rs2, rs1, 0)); |
There was a problem hiding this comment.
🟡 lo12 取低 16 位而非低 12 位
(val as i16) as i32 取 val 低 16 位并符号扩展,但 ADDI 立即数只有 12 位有符号(-2048~2047)。
当 val 的 bits[15:12] 非零时,lo12 会超出 ADDI 12 位范围,rv_i 中 imm << 20 的高位会污染指令的 rs1/funct3/rd 字段。
例如 val=0x1000: lo12=4096 (0x1000),imm << 20 = 0x1000_0000,bit[28] 溢出到 rs1 字段。
修复:let lo12 = ((val as i64) << 52 >> 52) as i32; 只符号扩展低 12 位。
| } | ||
| BPF_RSH => { | ||
| if use_imm { | ||
| let shamt = if is_64 { |
There was a problem hiding this comment.
🟡 BPF_DIV 除零时未清零 dst
当 src==0 时,BEQ 跳过 DIVU,dst 保持原值。但 Linux eBPF 规范要求 DIV by 0 返回 0。
需要在跳过路径中将 dst 清零。例如:
BEQ src, x0, +12 // skip to zeroing
DIVU dst, dst, src
JALR x0, x0, +8 // skip zeroing
ADDI dst, x0, 0 // dst = 0 (除零返回值)
注:BPF_MOD 除零保持 dst 不变的处理是正确的,不需要修改。
| emit_beq(buf, src, RV_ZERO, 0); | ||
| emit_remuw(buf, dst, dst, src); | ||
| } | ||
| let after = buf.offset(); |
There was a problem hiding this comment.
🟡 死代码:emit_andi 在 emit_sll 之后执行,对已完成的移位结果无影响。RV64 硬件 SLL 只取 src 低 6 位,无需软件掩码。建议移除此行。BPF_RSH 和 BPF_ARSH 的 64 位寄存器源模式也有同样问题。
There was a problem hiding this comment.
PR #891 第五轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在之前四轮 review 基础上持续修复。本次审查基于当前 HEAD(acb1a673c)。
已修复的旧问题 ✅
emit_load_imm6464位立即数加载:已改用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 ✅emit_call寄存器 shuffle:已正确映射 r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅JitBuffer的Send/Sync实现 ✅- 模块重组到
ebpf/ebpf_jit/子目录,re-export 替代super::super::✅ rv_rw辅助函数替代 XOR opcode 切换 ✅- 除零分支偏移
*2已移除 ✅ BPF_EXIT死代码清理 ✅
🔴 阻塞性问题(本轮新发现)
1. BPF_DIV/MOD 除零保护逻辑错误——dst 在除法前被清零,除法结果永远为 0
当前代码:
emit_addi(buf, dst, RV_ZERO, 0); // dst = 0 ← 提前清零!
emit_beq(buf, src, RV_ZERO, 0); // if src==0, skip
emit_divu(buf, dst, dst, src); // dst = 0 / src = 0 ← 原始 dst 已丢失结果:当 src != 0 时,DIVU 计算 0 / src = 0,而非 原始dst / src。这意味着所有使用 BPF_DIV 的 eBPF 程序在 JIT 模式下除法结果恒为 0。
正确的实现模式:
// BPF_DIV 语义:divide by zero 返回 0
BEQ src, x0, +s // sk = skip_zeroing
DIVU dst, dst, src // dst = dst / src
JAL skip2 // skip_zeroing
ADDI dst, x0, 0 // src==0 时返回 0同样,BPF_MOD 的实现也有相同问题。按 eBPF 规范,MOD by zero 应保持 dst 不变(而非清零):
// BPF_MOD 语义:modulo by zero 保持 dst 不变
BEQ src, x0, +s // sk = skip_mod
REMU dst, dst, src // dst = dst % src
// src==0 时 dst 保持原值2. 所有跳转使用绝对地址而非 PC 相对地址,运行时跳转到错误位置
emit_jalr(buf, RV_ZERO, RV_ZERO, offset) // JALR x0, x0, offsetJALR 计算目标:PC = (x[rs1] + imm) & ~1 = (0 + offset) & ~1 = offset
但 offset 是 JIT buffer 内部的相对位置,不是绝对内存地址。JIT buffer 通过 alloc_zeroed 在堆上分配,运行时地址为 buf.ptr,而非 0。因此所有跳转(JA、条件跳转 JEQ/JGT/JNE 等)都会跳转到 offset 而非 buf.ptr + offset,跳转目标完全错误。
正确的实现应使用 PC 相对跳转:
// BPF_JA (unconditional jump) - PC 相对
AUIPC t6, %pcrel_hi(pdist_offset) // t6 = PC + hi20 of pdist_offset
JALR x0, t6, %pcrel_lo(pdist_offset) // PC = t6 + lo12 of pdist_offset或者对于近跳转,直接使用 JAL(PC 相对,±1MB 范围)。
3. 与 PR #888 存在完整代码重叠
- PR #891 的前三个 commit 包含 PR #888(同作者 CN-TangLin)的全部 eBPF 子系统内容
- 建议先合并 #888,然后本 PR rebase 到 dev 分支,仅保留 JIT 相关 commit
- 避免两个 PR 同时修改相同文件产生合并冲突
🟡 建议改进
4. 缺少 JIT 编译结果的单元测试
当前没有任何测试来验证 JIT 生成的 RISC-V 指令序列的正确性。建议添加针对性的单元测试,对每个指令翻译结果做 golden-match 比对。如 @Godones 所建议,可以考虑将 riscv64 后端贡献到 rbpf 上游仓库,利用其现有数百个测试用例验证正确性。
验证状态
- ✅
cargo fmt --check通过 - ✅
cargo xtask clippy --package starry-kernel:13/13 feature 配置全部通过 ⚠️ 缺少 JIT 编译结果的单元测试⚠️ 无 QEMU 运行验证(当前 sys_bpf 已路由到 eBPF 模块)
Duplicate/Overlap 分析
- PR #888(同作者):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 前3个commit包含其全部内容。
- 关系:personal-overlap
- 建议:先合并 #888,然后 rebase 本 PR 仅保留 JIT commit
- 未发现其他 eBPF JIT 相关的 open PR
- 搜索词:ebpf, jit, bpf, riscv64
Powered by glm-5.1
| emit_addi(buf, rd, rd, lo12); | ||
| } | ||
| } else { | ||
| let lo32 = val as u32 as i32; |
There was a problem hiding this comment.
除零保护逻辑缺陷:
emit_addi(buf, dst, RV_ZERO, 0) // dst = 0
emit_beq(buf, src, RV_ZERO, 0) // if src==0, skip to after
emit_divu(buf, dst, dst, src) // dst = 0 / src = 0ADDI dst, x0, 0 在除法之前就清零了 dst,导致 DIVU 计算出的结果是 0 / src = 0,原始 dst 值已丢失。这意味着所有包含 BPF_DIV 的 eBPF 程序在 JIT 模式下除法结果恒为 0。
正确的实现:
// BPF_DIV 语义:divide by zero 返回 0,否则 dst = dst / src
BEQ src, x0, +s // sk = skip_zeroing
DIVU dst, dst, src // dst = dst / src
JAL skip2 // skip_zeroing
ADDI dst, x0, 0 // src==0 时返回 0BPF_MOD 也存在同样问题。按 eBPF 规范,MOD by zero 应保持 dst 不变,而非清零:
// BPF_MOD 语义:modulo by zero 保持 dst 不变
BEQ src, x0, +s // sk = skip_mod
REMU dst, dst, src // dst = dst % src
// src==0 时 dst 保持原值| emit_srai(buf, dst, dst, shamt); | ||
| } else { | ||
| emit_sraiw(buf, dst, dst, shamt); | ||
| } |
There was a problem hiding this comment.
跳转使用了绝对地址而非 PC 相对地址。
emit_jalr(buf, RV_ZERO, RV_ZERO, offset) // JALR x0, x0, offsetJALR 计算目标:PC = (x[rs1] + imm) & ~1 = (0 + offset) & ~1 = offset
但 offset 是 JIT buffer 内部的相对位置偏移,不是绝对内存地址。JIT buffer 通过 alloc_zeroed 在堆上分配(参见 mod.rs),运行时地址为 buf.ptr,远非地址 0。因此跳转目标完全错误,JIT 代码在运行时会跳转到无效地址。
正确的实现应使用 PC 相对跳转:
// BPF_JA (unconditional jump) - PC 相对
AUIPC t6, %pcrel_hi(pdist_offset) // t6 = PC + hi20 of pdist_offset
JALR x0, t6, %pcrel_lo(pdist_offset) // PC = t6 + lo12 of pdist_offset或者对于近跳转,直接使用 JAL(PC 相对,±1MB 范围)。
所有条件跳转(JEQ/JGT/JGE/JNE 等)中的 emit_jalr(buf, RV_ZERO, RV_ZERO, branch_off) 也存在相同问题。
| } else { | ||
| emit_sraiw(buf, dst, dst, shamt); | ||
| } | ||
| } else if is_64 { |
There was a problem hiding this comment.
条件跳转中的无条件跳转也使用了绝对地址,与上方 JA 同一问题,详见上一条评论。
| .field("size", &self.size) | ||
| .field("pos", &self.pos) | ||
| .finish() | ||
| } |
There was a problem hiding this comment.
emit_u32 函数只有 #[cfg(target_arch = "riscv64")],但 rv_r 系列编码函数和 emit_add 等指令 emit 函数并没有架构门控。如果未来添加其他架构后端,这些通用函数会与架构特定的函数产生命名冲突或链接问题。建议考虑架构隔离策略,或将 emit 函数移到后端模块内。
There was a problem hiding this comment.
PR #891 第六轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在之前五轮 review 基础上持续修复,本次审查基于 HEAD acb1a673c。许多之前的问题已修复(emit_call 寄存器 shuffle、emit_load_imm64 64位常量加载、rv_rw 辅助函数、模块重组、Send/Sync 等),但本轮发现了 两个新的阻塞性 bug。
🔴 阻塞性问题
1. 所有跳转使用绝对地址而非 PC 相对地址——JIT 生成的代码完全无法正确跳转
emit_jmp 中所有跳转(BPF_JA 无条件跳转、所有条件跳转 JEQ/JGT/JNE 等)均使用 emit_jalr(buf, RV_ZERO, RV_ZERO, offset) 模式。这生成 JALR x0, x0, offset,计算目标地址为 (0 + offset) & ~1 = offset,即绝对地址。但 offset 是 JIT buffer 内部的相对偏移,不是绝对内存地址。JIT buffer 通过 alloc_zeroed 在堆上分配,运行时地址为 buf.ptr(非零),因此所有跳转都会跳转到接近地址 0 的位置,而非 buffer 内的正确目标。
正确的实现应使用 PC 相对跳转:
- 近跳转(±1MB):使用
JAL x0, offset(opcode 0x6F,PC 相对) - 远跳转:使用
AUIPC rd, %pcrel_hi(offset)+JALR x0, rd, %pcrel_lo(offset)
2. BPF_DIV/BPF_MOD 在分支前清零 dst,除法/取模结果恒为 0
emit_addi(buf, dst, RV_ZERO, 0) 在 BEQ 分支之前执行,将 dst 清零。当 src != 0 时,DIVU 计算 0 / src = 0(原始 dst 已丢失),REMU 计算 0 % src = 0。按 eBPF 规范,DIV by 0 应返回 0,MOD by 0 应保持 dst 不变。
正确模式:
BEQ src, x0, +skip_zero // 如果 src==0,跳到清零
DIVU dst, dst, src // dst = dst / src(正常)
J skip_done // 跳过清零
ADDI dst, x0, 0 // dst = 0(除零情况)
🟡 建议改进
3. 缺少 JIT 编译结果的单元测试
当前没有任何测试验证 JIT 生成的 RISC-V 指令序列正确性。建议添加 golden-match 单元测试,或考虑将 riscv64 后端贡献到 rbpf 上游利用其测试用例验证。
📝 已修复的旧问题 ✅
- emit_call 寄存器 shuffle:r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅
- emit_load_imm64:使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 ✅
- lo12 计算修正为
<<52>>5212 位符号扩展 ✅ - JitBuffer Send/Sync ✅
- 模块重组到 ebpf/ebpf_jit/ 子目录 ✅
- rv_rw 辅助函数 ✅
- BPF_LSH 寄存器移位 ANDI 移到 SLL 之前 ✅
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础。本 PR 前 3 个 commit 包含 #888 的全部内容。关系:personal-overlap。建议先合并 #888,再 rebase 本 PR 仅保留 JIT 相关 commit。
- 未发现其他 eBPF JIT 相关 open PR。
验证状态
- ✅
cargo fmt --check通过 - ✅
cargo xtask clippy --package starry-kernel:13/13 feature 配置全部通过 ⚠️ 缺少 JIT 编译结果单元测试⚠️ 无 QEMU 运行验证
Powered by glm-5.1
| let off = offsets[target_pc] as isize - buf.offset() as isize; | ||
| let off_words = (off / 4) as i32; | ||
| if off_words >= -1048576 && off_words <= 1048575 { | ||
| let jal_off = (off as i32) & !3; |
There was a problem hiding this comment.
这里 emit_jalr(buf, RV_ZERO, RV_ZERO, jal_off) 生成 JALR x0, x0, jal_off,计算目标地址为 (0 + jal_off) & ~1 = jal_off,即绝对地址跳转到接近 0 的地址。
但 jal_off 是 JIT buffer 内部的相对偏移,JIT buffer 通过 alloc_zeroed 分配在堆上(非零地址),因此实际跳转目标完全错误。
正确做法应使用 PC 相对跳转:
- 近跳转(±1MB):使用
JAL x0, offset(opcode 0x6F),或手动编码rv_j(offset)辅助函数 - 远跳转:使用
AUIPC T6, %pcrel_hi(offset)+JALR x0, T6, %pcrel_lo(offset)
或者更简单的方案:因为编译时 buffer 已分配,可以在 JitCompiler 中保存 buf.ptr 基地址,在 emit_jmp 中计算绝对目标地址 buf.ptr + offsets[target_pc],然后用 emit_load_imm64 + emit_jalr(x0, T6, 0) 跳转。
| let jal_off = (off as i32) & !3; | ||
| emit_jalr(buf, RV_ZERO, RV_ZERO, jal_off); | ||
| } else { | ||
| emit_load_imm64(buf, RV_T6, off as u64); |
There was a problem hiding this comment.
远跳转路径同样有问题:T6 = off(相对偏移),JALR x0, T6, 0 跳转到绝对地址 off(接近 0),而非 PC 相对目标。应加载绝对目标地址 buf.ptr + offsets[target_pc],或改用 AUIPC+JALR 的 PC 相对模式。
| }; | ||
|
|
||
| match op { | ||
| BPF_JEQ => { |
There was a problem hiding this comment.
条件跳转的 taken 路径也使用了 emit_jalr(buf, RV_ZERO, RV_ZERO, branch_off),同样是绝对地址跳转,目标完全错误。前面的 BNE/BGEU 等分支是 PC 相对的(B-type),跳过 8 字节到下一条指令是正确的;但 taken 路径的 JALR 跳转到错误地址。
正确做法:使用 PC 相对跳转。例如条件跳转可以用反向条件分支跳过 JAL 指令:
// JEQ 语义:如果 dst == src,跳转到 target
BNE dst, src, +8 // 反向条件,跳过下一条
JAL x0, branch_off // PC 相对无条件跳转但 RISC-V 的 JAL immediate 只有 21 位(±1MB 范围)。如果需要更大范围,仍然需要 AUIPC+JALR。
| } | ||
| } | ||
| BPF_DIV => { | ||
| emit_addi(buf, dst, RV_ZERO, 0); |
There was a problem hiding this comment.
这里 emit_addi(buf, dst, RV_ZERO, 0) 在分支之前将 dst 清零。当 src != 0 时,后续 emit_divu(buf, dst, dst, src) 计算 0 / src = 0,原始 dst 值已丢失。所有使用 BPF_DIV 的 eBPF 程序在 JIT 模式下除法结果恒为 0。
正确的实现模式:
// BPF_DIV 语义:divide by zero 返回 0
let skip_zero = buf.offset(); // 记录零检测位置
emit_beq(buf, src, RV_ZERO, 0); // if src==0, skip to zeroing
if is_64 { emit_divu(buf, dst, dst, src); }
else { emit_divuw(buf, dst, dst, src); }
let skip_done = buf.offset();
emit_jalr(buf, RV_ZERO, RV_ZERO, 8); // skip zeroing (注意:此处也需要改为 PC 相对)
emit_addi(buf, dst, RV_ZERO, 0); // dst = 0 (除零情况)
// 修补两个分支...注意 insn_size 中 BPF_DIV/BPF_MOD 当前估算为 12 字节,需要同步增加到 20 字节(BEQ + DIVU + J + ADDI + 两个 patch)。
| } | ||
| } | ||
| BPF_MOD => { | ||
| emit_addi(buf, dst, RV_ZERO, 0); |
There was a problem hiding this comment.
BPF_MOD 存在相同问题:dst 在分支前被清零,REMU 计算 0 % src = 0。按 eBPF 规范,MOD by zero 应保持 dst 不变,而非清零。
正确模式:
// BPF_MOD 语义:modulo by zero 保持 dst 不变
BEQ src, x0, +skip_mod // 如果 src==0,跳过取模(dst 保持原值)
REMU dst, dst, src // 正常取模这比 DIV 更简单——不需要零值分支,只需在 src==0 时跳过 REMU 即可。insn_size 估算保持 12 字节即可(BEQ + REMU + patch)。
d890f82 to
68d2ca0
Compare
There was a problem hiding this comment.
PR #891 第七轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前六轮 review 基础上持续修复。本次审查基于当前 HEAD(68d2ca0b)。许多之前的问题已修复(AUIPC PC 相对跳转、emit_call 寄存器 shuffle、emit_load_imm64、Send/Sync、模块重组等),但本轮发现了 3 个新的阻塞性 bug。
🔴 阻塞性问题
1. BPF_DIV 除法前将 dst 清零,除法结果恒为 0
emit_addi(buf, dst, RV_ZERO, 0); // dst = 0 ← 提前清零!
let skip = buf.offset();
emit_beq(buf, src, RV_ZERO, 0); // if src==0, skip div
emit_divu(buf, dst, dst, src); // dst = 0 / src = 0 ← 原始 dst 已丢失当 src != 0 时,DIVU 计算 0 / src = 0,原始 dst 已被覆盖。所有 BPF_DIV 结果恒为 0。
正确模式(参考 Linux riscv64 JIT):
BEQ src, x0, +skip_zero // src==0 → 跳到清零
DIVU dst, dst, src // dst = dst / src(正常)
JAL skip_done // 跳过清零
ADDI dst, x0, 0 // dst = 0(除零情况)
2. BPF_MOD 取模前将 dst 清零
同样的 bug。按 eBPF 规范,MOD by 0 应保持 dst 不变。但当前代码 emit_addi(dst, ZERO, 0) 后接 BEQ+REMU,导致:src != 0 时 0 % src = 0(原始 dst 丢失),src == 0 时 dst = 0(应保持不变)。
正确模式:
BEQ src, x0, +skip_mod // src==0 → 不做取模,保持 dst
REMU dst, dst, src // dst = dst % src(正常)
3. 条件跳转中 BNE/BGEU 等硬编码跳过偏移 28 字节,不匹配变长 load_imm64 序列
所有条件跳转使用以下模式:
emit_bne(buf, dst, src, 28); // 硬编码跳过 28 字节
emit_auipc(buf, RV_T6, 0); // 4 字节
emit_load_imm64(buf, RV_T1, off); // 4/8/24 字节(变长!)
emit_add(buf, RV_T6, RV_T6, RV_T1); // 4 字节
emit_jalr(buf, RV_ZERO, RV_T6, 0); // 4 字节fall-through 序列的实际长度:
- load_imm64 = 4 字节时:总计 16 字节,跳过 28 → 多跳 12 字节,落入后续指令中间
- load_imm64 = 8 字节时:总计 20 字节,跳过 28 → 多跳 8 字节
- load_imm64 = 24 字节时:总计 36 字节,跳过 28 → 少跳 8 字节,落入 ADD/JALR 中间
三种情况下跳过偏移都不正确,条件跳转的「不跳转」路径会执行到错误位置。
修复方案:使用与 insn_size 一致的固定长度序列(如始终使用最坏情况 24 字节的 load_imm64),或在 emit 时动态计算实际序列长度并回填 BNE 偏移。
🟡 建议改进
4. insn_size 估算与实际代码长度不一致的风险
insn_size 对条件跳转估算为 imm_size + zext_size + 40(最坏情况),但实际 emit 代码长度可能远小于此(如 load_imm64 只需 4 字节时,序列仅 16 字节而非 40 字节)。当实际代码比估算短时,offsets[] 中的值偏大,后续指令的跳转偏移计算 offsets[target] - buf.offset() 会偏大,可能导致跳转到错误位置。
建议:确保 insn_size 返回的值精确匹配实际 emit 的代码长度(如 load_imm64 始终使用最坏情况编码),或者改用三遍编译策略。
5. 缺少 JIT 编译结果单元测试
目前没有任何测试验证 JIT 生成的 RISC-V 指令序列正确性。建议添加 golden-match 单元测试,或如 @Godones 建议将 riscv64 后端贡献到 rbpf 上游仓库利用其数百个测试用例验证。
✅ 已修复的旧问题
- AUIPC PC 相对跳转替代了之前的绝对地址跳转 ✅
- emit_call 寄存器 shuffle:r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅
- emit_load_imm64:使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 ✅
- lo12 计算修正为
<<52>>5212 位符号扩展 ✅ - JitBuffer Send/Sync ✅
- 模块重组到 ebpf/ebpf_jit/ ✅
- rv_rw 辅助函数 ✅
- 除零分支偏移
*2移除 ✅ - BPF_LSH 寄存器源 ANDI 移到 SLL 之前 ✅
重叠分析
- PR #888(同作者):eBPF 子系统基础。本 PR 前 2 个 commit 包含其内容。关系:partial-overlap。建议先合并 #888,再 rebase 本 PR。
- PR #892(同作者):x86_64 JIT 后端,依赖本 PR。关系:有序依赖链。
- PR #893(同作者):aarch64 JIT 后端,依赖 #892。关系:有序依赖链。
验证状态
- ✅
cargo fmt --check通过(PR 声明) - ✅
cargo xtask clippy --package starry-kernel:13/13 feature 配置通过(PR 声明) ⚠️ CI check runs 全部 skipped,无实际 CI 验证⚠️ 缺少 JIT 编译结果单元测试- ❌ DIV/MOD 代码生成 bug 会导致所有使用除法/取模的 eBPF 程序在 JIT 模式下结果恒为 0
- ❌ 条件跳转 BNE 硬编码偏移会导致「不跳转」路径跳转到错误位置
Powered by glm-5.1
| } | ||
| BPF_LSH => { | ||
| if use_imm { | ||
| let shamt = if is_64 { |
There was a problem hiding this comment.
🔴 BPF_DIV 除法前清零 bug
emit_addi(buf, dst, RV_ZERO, 0) 在 BEQ 之前执行,将 dst 清零。当 src != 0 时,DIVU 计算 0 / src = 0,原始 dst 已丢失。
正确模式(参考 Linux arch/riscv/net/bpf_jit_comp64.c):
BEQ src, x0, +skip_zero // src==0 → 跳到清零
DIVU dst, dst, src // dst = dst / src
JAL skip_done // 跳过清零
ADDI dst, x0, 0 // dst = 0(除零时)
| } | ||
| } | ||
| BPF_RSH => { | ||
| if use_imm { |
There was a problem hiding this comment.
🔴 BPF_MOD 取模前清零 bug
同 BPF_DIV 的 bug。emit_addi(dst, ZERO, 0) 后接 BEQ+REMU,导致:
- src != 0 时:
0 % src = 0(原始 dst 丢失) - src == 0 时:dst = 0(按 eBPF 规范应保持 dst 不变)
正确模式:
BEQ src, x0, +skip_mod // src==0 → 不做取模
REMU dst, dst, src // dst = dst % src
| RV_T1 | ||
| } else { | ||
| bpf_to_rv(insn.src_reg()) | ||
| }; |
There was a problem hiding this comment.
🔴 BNE 硬编码跳过偏移 28 字节不正确
fall-through 序列为 AUIPC(4) + load_imm64(?) + ADD(4) + JALR(4):
- load_imm64 = 4 字节时:总计 16 字节,28 多跳 12 字节
- load_imm64 = 8 字节时:总计 20 字节,28 多跳 8 字节
- load_imm64 = 24 字节时:总计 36 字节,28 少跳 8 字节
三种情况下都不匹配,「不跳转」路径会执行到错误位置。
建议:让 load_imm64 始终使用最坏情况(24 字节)编码,使 fall-through 长度固定为 36 字节;或动态记录 emit 前后的 offset 差值来回填 BNE 偏移。此问题影响所有 11 种条件跳转。
68d2ca0 to
07e7ca9
Compare
There was a problem hiding this comment.
PR #891 第N轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前几轮 review 基础上持续修复。本轮基于当前 HEAD(07e7ca9ad)进行审查。许多之前的问题已修复(emit_call 寄存器 shuffle、emit_load_imm64 64位常量加载、Send/Sync、模块重组、rv_rw、PC相对跳转等),但本轮仍发现了 2 个阻塞性 bug。
🔴 阻塞性问题
1. BPF_DIV/BPF_MOD 在分支检查前将 dst 清零——除法/取模结果恒为 0
当前 BPF_DIV 代码(jit_riscv64.rs 第 429-444 行):
emit_addi(buf, dst, RV_ZERO, 0); // dst = 0 ← 在 BEQ 之前执行!
let skip = buf.offset();
emit_beq(buf, src, RV_ZERO, 0); // 若 src==0,跳过除法
emit_divu(buf, dst, dst, src); // dst = 0 / src = 0 ← 原始 dst 已丢失当 src != 0 时,ADDI dst, x0, 0 已把 dst 清零,DIVU 计算 0 / src = 0。所有非零除数的除法结果恒为 0。
BPF_DIV 语义:除数为 0 时返回 0,除数非 0 时正常计算 dst / src。
正确模式:
BEQ src, x0, +12 // 若 src==0,跳到清零
DIVU dst, dst, src // 正常除法:dst = dst / src
JAL x0, +8 // 跳过清零
ADDI dst, x0, 0 // 除零情况:dst = 0
BPF_MOD 有相同的 bug(第 507 行),且按 eBPF 规范 MOD by 0 应保持 dst 不变(不需要 ADDI 清零):
BEQ src, x0, +8 // 若 src==0,跳过取模,dst 保持不变
REMU dst, dst, src // 正常取模
2. 条件跳转中 BNE/BEQ 硬编码跳过 28 字节,不匹配变长 emit_load_imm64
emit_jmp 中所有条件跳转(第 612 行及后续)使用以下模式:
emit_bne(buf, dst, src, 28); // 硬编码跳过 28 字节
emit_auipc(buf, RV_T6, 0); // 4 字节
emit_load_imm64(buf, RV_T1, branch_off); // 4/8/24 字节(变长!)
emit_add(buf, RV_T6, RV_T6, RV_T1); // 4 字节
emit_jalr(buf, RV_ZERO, RV_T6, 0); // 4 字节fall-through 序列的实际长度取决于 branch_off 值:
branch_off在 [-2048, 2048) →emit_load_imm644 字节 → 总计 16 字节。BNE 跳过 28 → 多跳 12 字节,落入后续指令中间branch_off在 32 位范围内 →emit_load_imm648 字节 → 总计 20 字节。跳过 28 → 多跳 8 字节branch_off需要完整 64 位 →emit_load_imm6424 字节 → 总计 36 字节。跳过 28 → 少跳 8 字节,执行到 ADD/JALR 中间
三种情况下跳过偏移都不正确。当条件不成立时,BNE/BEQ 的 fall-through 路径会跳转到错误位置。
修复建议:
- 方案 A:在 insn_size 和 emit 中使用固定长度的跳转序列(始终用 24 字节的完整 64 位加载路径),然后条件跳转 skip 偏移用 36
- 方案 B:动态计算实际序列长度并回填 BNE/BEQ 偏移(类似除零保护的做法)
🟡 建议改进
3. 缺少 JIT 编译结果的单元测试
当前没有任何测试验证 JIT 生成的 RISC-V 指令序列的正确性。考虑到上述代码生成 bug 的存在,强烈建议添加针对性的单元测试,至少覆盖:
- 各 ALU 操作的正确编码
- 除零保护的正确分支跳转
- 条件跳转的 fall-through 路径
- helper 调用的寄存器映射
✅ 已修复的问题(确认)
- emit_call 寄存器 shuffle:r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅
- emit_load_imm64:使用
<<52>>52正确符号扩展低 12 位 ✅ - JitBuffer Send/Sync + Debug ✅
- 模块重组到 ebpf/ebpf_jit/ ✅
- rv_rw 辅助函数替代 XOR opcode 切换 ✅
- 跳转使用 AUIPC+load_imm64+ADD+JALR PC 相对模式 ✅
- BPF_LSH 寄存器源 ANDI 移到 SLL 之前 ✅
- BPF_EXIT 在 compile() 中处理,emit_jmp 无死代码 ✅
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 前 6 个 commit 包含 #888 的全部内容(Head:
78c6d632)。 - 未发现其他 eBPF JIT 相关的 open PR
- 搜索词:ebpf, jit, bpf, riscv64
验证状态
- ✅
cargo fmt --check:通过 - ✅
cargo xtask clippy --package starry-kernel:14/14 feature 配置全部通过 ⚠️ CI 全部跳过(fork PR 安全策略,非 PR 相关)⚠️ 缺少 JIT 编译结果单元测试⚠️ 无 QEMU 运行验证(但 JIT 代码路径与 sys_bpf 已正确集成)
Powered by deepseek-v4-pro
| } | ||
| } | ||
| BPF_DIV => { | ||
| emit_addi(buf, dst, RV_ZERO, 0); |
There was a problem hiding this comment.
🔴 BPF_DIV 除零保护 bug:emit_addi(buf, dst, RV_ZERO, 0) 在 BEQ 分支检查之前执行,将 dst 清零。当 src != 0 时,DIVU 计算 0 / src = 0(原始 dst 已丢失),而非 原始dst / src。
正确的模式应是将 ADDI dst, x0, 0 放在 BEQ 跳转的目标位置(除零分支),而非在所有路径中执行:
BEQ src, x0, +12 // src==0 → 跳到清零
DIVU dst, dst, src // 正常情况:dst = dst / src
JAL x0, +8 // 跳过清零
ADDI dst, x0, 0 // 除零情况:dst = 0
| } | ||
| } | ||
| BPF_MOD => { | ||
| emit_addi(buf, dst, RV_ZERO, 0); |
There was a problem hiding this comment.
🔴 BPF_MOD 同样有 dst 提前清零的 bug。且按 eBPF 规范,MOD by zero 应保持 dst 不变,不需要 ADDI 清零。正确模式只需 BEQ 跳过 REMU:
BEQ src, x0, +8 // src==0 → 不做取模,dst 保持不变
REMU dst, dst, src // 正常情况:dst = dst % src
|
|
||
| match op { | ||
| BPF_JEQ => { | ||
| emit_bne(buf, dst, src, 28); |
There was a problem hiding this comment.
🔴 条件跳转硬编码 skip 偏移 28 不匹配变长 load_imm64。emit_bne(buf, dst, src, 28) 中的 28 假设 emit_load_imm64 始终产生 16 字节的 fall-through 序列(AUIPC 4 + load_imm64 8 + ADD 4 + JALR 4 = 20,但 28 也不对)。
emit_load_imm64 实际产出的序列长度取决于 branch_off 值:
- 4 字节(branch_off ∈ [-2048,2048)):fall-through = 16 字节,跳过 28 → 多跳 12 字节
- 8 字节(branch_off 在 32 位范围):fall-through = 20 字节,跳过 28 → 多跳 8 字节
- 24 字节(完整 64 位加载):fall-through = 36 字节,跳过 28 → 少跳 8 字节
三种情况下跳转偏移均不正确。建议使用固定长度序列(始终 36 字节)或动态回填 BNE 偏移。同一问题影响所有 11 种条件跳转(JEQ/JGT/JGE/JSET/JNE/JSGT/JSGE/JLT/JLE/JSLT/JSLE)。
There was a problem hiding this comment.
PR #891 审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在之前多轮 review 基础上持续修复。本次审查基于当前 HEAD (1e1c46ce4)。许多之前的问题已修复(emit_call 寄存器 shuffle、emit_load_imm64 64位常量加载、PC 相对跳转、rv_rw 辅助函数、模块重组、Send/Sync 等),但本轮仍发现了 3 个阻塞性 bug。
🔴 阻塞性问题
1. BPF_DIV 除法前将 dst 清零,所有除法结果恒为 0
emit_addi(buf, dst, RV_ZERO, 0); // dst = 0 ← 提前清零!
let skip = buf.offset();
emit_beq(buf, src, RV_ZERO, 0); // if src==0, skip div
emit_divu(buf, dst, dst, src); // dst = 0 / src = 0 ← 原始 dst 已丢失当 src != 0 时,DIVU 计算 0 / src = 0,原始 dst 已被覆盖。所有 BPF_DIV 结果恒为 0。
正确模式(参考 Linux riscv64 JIT):
BEQ src, x0, +skip_zero // src==0 → 跳到清零
DIVU dst, dst, src // dst = dst / src(正常情况)
J skip_done // 跳过清零代码
skip_zero:
ADDI dst, x0, 0 // dst = 0(除零情况)
skip_done:
详见 inline comment。
2. BPF_MOD 取模前将 dst 清零
同样的 bug。按 eBPF 规范,MOD by 0 应保持 dst 不变。但当前代码 emit_addi(dst, ZERO, 0) 后接 BEQ+REMU,导致:
src != 0时:0 % src = 0(原始 dst 丢失)src == 0时:dst = 0(应保持原值不变)
正确模式:
BEQ src, x0, +skip_mod // src==0 → 不做取模,保持 dst
REMU dst, dst, src // dst = dst % src(正常情况)
skip_mod:
详见 inline comment。
3. 条件跳转中 BNE/BGEU 等硬编码跳过偏移 28 字节,不匹配变长 load_imm64 序列
所有条件跳转使用以下模式:
emit_bne(buf, dst, src, 28); // 硬编码跳过 28 字节
emit_auipc(buf, RV_T6, 0); // 4 字节
emit_load_imm64(buf, RV_T1, off); // 4/8/24 字节(变长!)
emit_add(buf, RV_T6, RV_T6, RV_T1); // 4 字节
emit_jalr(buf, RV_ZERO, RV_T6, 0); // 4 字节fall-through 序列的实际长度:
emit_load_imm64= 4 字节时(offset 在 ±2KB 内):总计 16 字节,BNE 跳过 28 → 多跳 12 字节,落入后续指令中间emit_load_imm64= 8 字节时(offset 在 32-bit 内):总计 20 字节,BNE 跳过 28 → 多跳 8 字节emit_load_imm64= 24 字节时(完整 64-bit offset):总计 36 字节,BNE 跳过 28 → 少跳 8 字节,落入 ADD/JALR 中间
三种情况下跳过偏移都不正确,条件跳转的「不跳转」路径会执行到错误位置。
修复方案:
- 方案 A(推荐):仿照 BPF_DIV 的 patch 模式,在 emit 之后回填正确的分支偏移
- 方案 B:将 AUIPC+load_imm64+ADD+JALR 替换为固定长度序列(如对远跳转使用 10 条指令定长)
详见 inline comments。
🟡 建议改进
4. insn_size 对条件跳转的估算与 emit_jmp 实际 emit 不一致
insn_size 估算:imm_size + zext_size + 40 = 44 或 48 字节
emit_jmp 实际 emit:4 + load_imm64(N) + 4 + 4 + 4 = 16-40 字节
insn_size 在典型 case 中高估约 24 字节,导致 JIT buffer 分配过大但不会产生错误。不过建议使估算与实际一致,便于调试和内存使用优化。
5. 缺少 JIT 编译结果的单元测试
当前没有任何测试验证 JIT 生成的 RISC-V 指令序列的正确性。建议添加 golden-match 单元测试,对每个 BPF 指令的翻译结果做逐字节比对。可参考 Linux 内核 tools/testing/selftests/bpf/ 下的测试用例。
📝 已修复的旧问题 ✅
emit_call寄存器 shuffle:r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅emit_load_imm64:使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 ✅- lo12 计算修正为
<<52>>5212 位符号扩展 ✅ JitBufferSend/Sync ✅- 模块重组到
ebpf/ebpf_jit/子目录 ✅ rv_rw辅助函数替代 XOR opcode 切换 ✅- BPF_LSH 寄存器移位 ANDI 移到 SLL 之前 ✅
- 跳转使用 AUIPC PC 相对寻址 ✅
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 前 6 个 commit 包含 #888 的全部内容。
- 未发现其他 eBPF JIT 相关的 open PR
验证状态
- ✅
cargo fmt --check通过 - ✅
cargo xtask clippy --package starry-kernel:14/14 feature 配置全部通过 ⚠️ 缺少 JIT 编译结果的单元测试⚠️ 无 QEMU 运行验证(当前sys_bpf已路由到 eBPF 模块,但syscall/mod.rs中 ebpf feature 下的路由需要确认)
总结
PR 在之前的 review 基础上做了大量修复,emit_call、emit_load_imm64、跳转寻址等核心问题已解决。但当前代码仍存在 3 个阻塞性 bug:BPF_DIV/BPF_MOD 除零逻辑导致结果恒为 0,以及条件跳转的硬编码偏移不匹配变长指令序列。建议修复这 3 个问题后重新提交。
Powered by deepseek-v4-pro
| emit_lui(buf, rd, hi32_hi20); | ||
| emit_addiw(buf, rd, rd, hi32_lo12); | ||
| emit_slli(buf, rd, rd, 32); | ||
| let lo32 = val as u32 as i32; |
There was a problem hiding this comment.
🔴 阻塞性 bug:BPF_DIV 除法前将 dst 清零,所有除法结果恒为 0
当前代码流程:
emit_addi(buf, dst, RV_ZERO, 0)→ dst = 0(原始值被覆盖)emit_beq(buf, src, RV_ZERO, 0)→ 如果 src==0 跳过 DIVU,dst 保持 0emit_divu(buf, dst, dst, src)→ 如果 src!=0,计算 0/src = 0
两种情况下 dst 都是 0,原始值完全丢失。所有使用 BPF_DIV 的 eBPF 程序在 JIT 模式下结果都是错误的。
正确实现应参考 Linux riscv64 JIT(arch/riscv/net/bpf_jit_comp64.c 的 emit_alu()):
// 正确模式
let skip = buf.offset();
emit_beq(buf, src, RV_ZERO, 0); // 占位,稍后回填
emit_divu(buf, dst, dst, src); // 正常除法
after_div = buf.offset();
emit_jal(buf, RV_ZERO, 0); // 跳过清零代码,占位
after_skip = buf.offset();
emit_addi(buf, dst, RV_ZERO, 0); // 除零时 dst=0
after = buf.offset();
// 回填 BEQ: 跳转到 after_skip
// 回填 JAL: 跳转到 after注意:需要像现有的 patch 代码一样回填两个分支指令。
|
|
||
| fn emit_alu(buf: &mut JitBuffer, insn: &BpfInsn, is_64: bool) { | ||
| let dst = bpf_to_rv(insn.dst_reg()); | ||
| let use_imm = (insn.code & BPF_X) == 0; |
There was a problem hiding this comment.
🔴 阻塞性 bug:BPF_MOD 取模前将 dst 清零
与 BPF_DIV 相同的问题。按 eBPF 规范,BPF_MOD 除零时应保持 dst 不变。但当前代码将 dst 清零后,所有情况结果都错:
- src==0: dst=0(应保持原值)
- src!=0: dst = 0%src = 0(原始值丢失)
正确实现:
let skip = buf.offset();
emit_beq(buf, src, RV_ZERO, 0); // 占位
emit_remu(buf, dst, dst, src); // 正常取模
after = buf.offset();
// 回填 BEQ: 跳转到 after(跳过 REMU)
// 当 src==0 时,不执行 REMU,dst 保持原值这里比 BPF_DIV 简单——只需在 src==0 时跳过 REMU 即可,不需要额外清零。
| } | ||
| } | ||
| BPF_OR => { | ||
| if is_64 { |
There was a problem hiding this comment.
🔴 阻塞性 bug:条件跳转中 BNE 硬编码偏移 28 字节与变长 load_imm64 不匹配
此处(及以下所有条件跳转 JEQ/JGT/JGE/JSET/JNE/JSGT/JSGE/JLT/JLE/JSLT/JSLE)均使用 emit_bne(buf, dst, src, 28) 硬编码跳过 28 字节。
但 fall-through 序列的实际长度取决于 emit_load_imm64 生成的指令数:
- branch_off 在 ±2KB 内 → 1 条指令(4B) → AUIPC+ADDI+ADD+JALR = 16B → BNE 应为 20(非 28)→ 当前多跳 8B
- branch_off 在 32-bit 内 → 2 条指令(8B) → AUIPC+LUI+ADDI+ADD+JALR = 20B → BNE 应为 24(非 28)→ 当前多跳 4B
- branch_off 64-bit 范围 → 6 条指令(24B) → AUIPC+6条+ADD+JALR = 36B → BNE 应为 40(非 28)→ 当前少跳 12B
修复建议:仿照 BPF_DIV/BPF_MOD 的 patch 模式,先 emit 占位 BNE,待 fall-through 序列 emit 完成后回填正确偏移。这需要将 offset 记录为 usize 并 unsafe 写回。
| } else { | ||
| emit_orw(buf, dst, dst, src); | ||
| } | ||
| } |
There was a problem hiding this comment.
同上,emit_bgeu(buf, dst, src, 28) 硬编码 28 字节不正确,需回填实际 fall-through 长度。
| if is_64 { | ||
| emit_and(buf, dst, dst, src); | ||
| } else { | ||
| emit_andw(buf, dst, dst, src); |
There was a problem hiding this comment.
同上,emit_bltu(buf, dst, src, 28) 硬编码 28 字节不正确。
| } else { | ||
| emit_andi(buf, RV_T2, src, 31); | ||
| emit_sllw(buf, dst, dst, RV_T2); | ||
| } |
There was a problem hiding this comment.
同上,emit_blt(buf, dst, src, 28) 硬编码 28 字节不正确。
| BPF_RSH => { | ||
| if use_imm { | ||
| let shamt = if is_64 { | ||
| (insn.imm as u32) & 63 |
There was a problem hiding this comment.
同上,emit_bgeu(buf, dst, src, 28) 硬编码 28 字节不正确。
| (insn.imm as u32) & 31 | ||
| }; | ||
| if is_64 { | ||
| emit_srli(buf, dst, dst, shamt); |
There was a problem hiding this comment.
同上,emit_bltu(buf, dst, src, 28) 硬编码 28 字节不正确。
| emit_srliw(buf, dst, dst, shamt); | ||
| } | ||
| } else if is_64 { | ||
| emit_andi(buf, RV_T2, src, 63); |
There was a problem hiding this comment.
同上,emit_bge(buf, dst, src, 28) 硬编码 28 字节不正确。
| } else { | ||
| emit_andi(buf, RV_T2, src, 31); | ||
| emit_srlw(buf, dst, dst, RV_T2); | ||
| } |
There was a problem hiding this comment.
同上,emit_blt(buf, dst, src, 28) 硬编码 28 字节不正确。
There was a problem hiding this comment.
PR #891 第八轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
这是本 PR 的第八轮自动审查。感谢在前七轮基础上持续修复了大量问题(emit_call 寄存器 shuffle、emit_load_imm64 64位常量加载、AUIPC PC 相对跳转、rv_rw 辅助函数、Send/Sync、模块重组等均已正确)。cargo xtask clippy 14/14 feature 配置全部通过。
但当前 HEAD (66457040) 仍存在 2 个阻塞性代码生成 bug:
🔴 阻塞性问题
1. BPF_DIV/BPF_MOD 的 dst 在 BEQ 分支前被清零,除法/取模结果恒为 0
jit_riscv64.rs 第 429-444 行(BPF_DIV)和第 507-522 行(BPF_MOD):
emit_addi(buf, dst, RV_ZERO, 0); // dst = 0 ← 提前清零!
let skip = buf.offset();
emit_beq(buf, src, RV_ZERO, 0); // if src==0, skip div
emit_divu(buf, dst, dst, src); // dst = 0 / src = 0 ← 原始 dst 已丢失当 src != 0 时,DIVU 计算 0 / src = 0(原始 dst 被提前清零覆盖),REMU 计算 0 % src = 0。按 eBPF 规范,DIV by 0 应返回 0,MOD by 0 应保持 dst 不变。但当前:
- DVI by 0:dst 已提前清零,跳过 DIVU,结果 = 0(看似正确,但正常除法也返回 0 是 bug)
- DVI 正常:dst 已提前清零,DIVU 计算 0/src = 0(错误)
- MOD by 0:dst 已提前清零,跳过 REMU,结果 = 0(错误,应保持原值)
- MOD 正常:dst 已提前清零,REMU 计算 0%src = 0(错误)
正确模式(参考 Linux kernel riscv64 JIT):
BEQ src, x0, +skip ; src==0 → 跳过正常除法
DIVU dst, dst, src ; dst = dst / src(正常)
JAL done ; 跳过清零
ADDI dst, x0, 0 ; dst = 0(除零)
对于 MOD,除零时 dst 应保持不变,无需额外指令。
2. 条件跳转的 28 字节硬编码跳过偏移与变长 emit_load_imm64 不匹配
jit_riscv64.rs 第 612-683 行,所有条件跳转使用:
emit_bne(buf, dst, src, 28); // 硬编码跳过 28 字节
emit_auipc(...) // 4 字节
emit_load_imm64(...) // 4/8/24 字节(变长!)
emit_add(...) // 4 字节
emit_jalr(...) // 4 字节不匹配情况:
emit_load_imm644 字节(小立即数):序列总长 16 字节 → BNE 多跳 12 字节emit_load_imm648 字节(32位符号扩展):序列总长 20 字节 → BNE 多跳 8 字节emit_load_imm6424 字节(64位):序列总长 36 字节 → BNE 少跳 8 字节
三种情况都不正确,导致条件跳转的 fall-through 路径执行到错误位置。
修复方案:在 emit 时动态计算实际序列长度并回填分支偏移,或使用固定长度序列(始终 emit 24 字节 load_imm64)。
📝 已修复的旧问题 ✅
emit_call寄存器 shuffle:r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅emit_load_imm64:使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 ✅- AUIPC PC 相对跳转替代绝对地址 JALR ✅
rv_rw辅助函数、Send/Sync、模块重组 ✅lo12计算修正为<<52>>52✅- 除零分支偏移
*2移除 ✅
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 前 3 个 commit 包含 #888 的全部内容。两 PR 均 open。关系:partial-overlap,建议先合并 #888,再 rebase 本 PR。
- 搜索词 ebpf/jit/bpf/riscv64,未发现其他相关 open PR。
验证状态
- ✅
cargo xtask clippy --package starry-kernel:14/14 feature 配置全部通过 ⚠️ 缺少 JIT 编译结果单元测试⚠️ 无 QEMU 运行验证
Powered by deepseek-v4-pro
| } | ||
| } | ||
| BPF_DIV => { | ||
| emit_addi(buf, dst, RV_ZERO, 0); |
There was a problem hiding this comment.
🔴 BPF_DIV:emit_addi(buf, dst, RV_ZERO, 0) 在 BEQ 分支检查之前将 dst 清零。当 src != 0 时,DIVU 计算 0 / src = 0,原始 dst 已丢失。请将清零移到 BEQ 分支之后(仅在除零时清零),正常除法前不应清零 dst。
| } | ||
| } | ||
| BPF_MOD => { | ||
| emit_addi(buf, dst, RV_ZERO, 0); |
There was a problem hiding this comment.
🔴 BPF_MOD:同上,emit_addi(buf, dst, RV_ZERO, 0) 提前清零 dst。且 eBPF 规范规定 MOD by 0 应保持 dst 不变,不是返回 0。请移除这行 ADD,除零时 dst 自然保持不变。正常取模时 REMU 直接操作 dst,不应提前清零。
|
|
||
| match op { | ||
| BPF_JEQ => { | ||
| emit_bne(buf, dst, src, 28); |
There was a problem hiding this comment.
🔴 硬编码跳过偏移 28 字节与变长的 emit_load_imm64 不匹配(4/8/24 字节)。AUIPC(4) + load_imm64(4/8/24) + ADD(4) + JALR(4) 实际长度 16/20/36 字节。当 load_imm64=4 字节时多跳 12 字节,当 load_imm64=24 字节时少跳 8 字节。建议动态计算并回填偏移,或使用固定长度序列。
There was a problem hiding this comment.
PR #891 第八轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前七轮 review 基础上持续修复。本次审查基于当前 HEAD (2b731f77)。多个之前的问题已正确修复(emit_call 寄存器 shuffle ✅、BPF_DIV/MOD 除零处理 ✅、AUIPC PC 相对跳转 ✅、BPF_EXIT 处理 ✅、Send/Sync ✅、模块重组 ✅),但本轮发现了一个新的代码生成 bug。
🔴 阻塞性问题
1. emit_load_imm64 中 lo12 符号扩展计算有 bug
在 jit_riscv64.rs 中,三处 lo12 计算使用了 ((value as i64) << 52 >> 52) as i32 模式(行 325、334、340)。此模式在原始值的第 12 位及以上有非零位时,无法正确对低 12 位进行符号扩展。
问题根因:value as i64 << 52 将整个 64 位值(包括 bits[63:12])左移,而非仅取低 12 位。当 value 的 bit 12 及以上有非零位时,左移后的 bit 63 可能为 0,导致算术右移不进行正确的符号扩展。
例如 value = 0x1800 (6144) 时:
- 当前方法:
lo12 = 6144(错误) - 正确结果:
lo12 = -2048(0x800 的 12 位符号扩展) - 生成的指令:LUI rd, 0 → ADDI rd, rd, 0x800(ADDI 将 0x800 解释为 -2048),最终结果 = -2048,而非期望的 6144。
修复方案:将三处 lo12 计算改为标准的 32 位低 12 位符号扩展:
// 修复前:
let lo12 = (((lo32 as i64) << 52) >> 52) as i32;
// 修复后:
let lo12 = (lo32 << 20) >> 20;此模式利用 i32 的 << 20 将低 12 位移至 bits[31:20],再 >> 20(算术右移)进行符号扩展。对 hi32_lo12 和 lo32_lo12 同理。
🟡 建议改进
2. BPF_MOD 中 skip 变量未使用(行 509)
let skip = buf.offset(); 声明后从未使用,会产生 dead_code 警告。建议删除。
✅ 已修复的旧问题(确认)
- emit_call 寄存器 shuffle:A0←r1, A1←r2, A2←r3, A3←r4, A4←r5 ✅
- BPF_DIV 除零:BEQ→DIVU→JAL→ADDI 模式,动态回填分支偏移 ✅
- BPF_MOD 除零:BEQ +8 跳过 REMU,dst 保持不变 ✅
- 条件跳转:AUIPC+load_imm64+ADD+JALR PC 相对,动态回填 BNE/BEQ 偏移 ✅
- BPF_EXIT 在 compile() 中调用 emit_epilogue() ✅
- BPF_LSH/RSH/ARSH 寄存器源:ANDI 在移位之前执行 ✅
- JitBuffer Send/Sync + Debug ✅
- rv_rw 辅助函数替代 XOR opcode ✅
- 模块结构:ebpf/ebpf_jit/ 子目录 + re-export ✅
验证状态
- ✅
cargo fmt --check通过 - ✅
cargo clippy --features ebpf -- -D warnings通过(--all-features失败是 ax-hal/kcov 预存问题,与 PR 无关) ⚠️ CI 全部 skipped — 不是 PR 变更导致的⚠️ 缺少 JIT 编译结果的单元测试
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(2903+ 行)。PR #891 的前 7 个 commit 包含 #888 的全部内容。两个 PR 的 base 相同(dev
247c6d7ea)。 - 未发现其他 eBPF JIT 相关的 open PR
Powered by deepseek-v4-pro
| } | ||
| if (val as u32 as i32) as i64 == val_i { | ||
| let lo32 = val as u32 as i32; | ||
| let lo12 = (((lo32 as i64) << 52) >> 52) as i32; |
There was a problem hiding this comment.
🔴 lo12 符号扩展计算错误。((lo32 as i64) << 52 >> 52) 无法正确对低 12 位进行符号扩展(当 lo32 的 bit 12 及以上有非零位时)。例如 lo32=0x1800 (6144) 时,计算得 lo12=6144(应为 -2048)。应改为 let lo12 = (lo32 << 20) >> 20; 使用 i32 算术右移进行低 12 位符号扩展。
| } else { | ||
| let hi32 = (val >> 32) as u32 as i32; | ||
| let hi32_hi20 = ((hi32 as i64 + 0x800) >> 12) as u32; | ||
| let hi32_lo12 = (((hi32 as i64) << 52) >> 52) as i32; |
There was a problem hiding this comment.
🔴 与行 325 同样的 << 52 >> 52 问题。hi32_lo12 符号扩展不正确。应改为 let hi32_lo12 = (hi32 << 20) >> 20;。
| emit_slli(buf, rd, rd, 32); | ||
| let lo32 = val as u32 as i32; | ||
| let lo32_hi20 = ((lo32 as i64 + 0x800) >> 12) as u32; | ||
| let lo32_lo12 = (((lo32 as i64) << 52) >> 52) as i32; |
There was a problem hiding this comment.
🔴 与行 325 同样的 << 52 >> 52 问题。lo32_lo12 符号扩展不正确。应改为 let lo32_lo12 = (lo32 << 20) >> 20;。
| } | ||
| } | ||
| BPF_MOD => { | ||
| let skip = buf.offset(); |
There was a problem hiding this comment.
🟡 let skip = buf.offset(); 声明后从未使用,会产生 dead_code 警告。建议删除此行。
There was a problem hiding this comment.
PR #891 第八轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在之前七轮 review 基础上持续修复。本次审查基于当前 HEAD(db728813d)。许多旧问题已修复(emit_call 寄存器 shuffle、emit_load_imm64 64位常量加载、PC 相对跳转、模块重组、Send/Sync 等),但本轮发现了 3 个新的阻塞性 bug,这些在之前的 review 中未被识别。
🔴 阻塞性问题
1. BPF_MOD 除零时错误地将 dst 清零
当前代码对 BPF_MOD 和 BPF_DIV 使用相同的除零处理序列(BEQ → REMU/DIVU → JAL → ADDI dst,0)。按 eBPF 规范,DIV by 0 应返回 0(✅ 正确),但 MOD by 0 应保持 dst 不变(❌ 当前错误清零)。
RISC-V 硬件 REMU by 0 本身就会返回被除数(dst 不变),因此 MOD 的最简修复是直接去掉 JAL+ADDI 序列,只保留 BEQ src, zero, skip_rem + REMU dst, dst, src(或更简单地直接只用 REMU dst, dst, src 依靠硬件行为,但显式 BEQ 更安全可移植)。
参考:Linux 内核 kernel/bpf/verifier.c 规定 "BPF_MOD: if src == 0, dst stays unchanged"。
2. 4 个条件跳转的反向条件操作数互换
所有条件跳转使用「反向条件分支 + 跳转块」模式:先 emit 一个反向条件分支跳过跳转块,再 emit 跳转块,最后回填分支偏移。但以下 4 个条件跳转的操作数顺序错误:
| BPF 操作 | eBPF 语义 | 所需反向条件 | 当前 emit | 正确 emit |
|---|---|---|---|---|
| JGT | dst > src (u) | dst ≤ src | BGEU dst, src |
BGEU src, dst |
| JSGT | dst > src (s) | dst ≤ src | BGE dst, src |
BGE src, dst |
| JLE | dst ≤ src (u) | dst > src | BLTU dst, src |
BLTU src, dst |
| JSLE | dst ≤ src (s) | dst > src | BLT dst, src |
BLT src, dst |
以 JGT 为例:当 dst=5, src=3 时,dst > src 应为真(应跳转)。但 BGEU dst, src 的分支条件为 5 ≥ 3 → 真 → 分支跳过跳转块 → 错误地不跳转。当 dst=3, src=5 时,dst > src 应为假(不应跳转),但 BGEU 3,5 不分支 → 落入跳转块 → 错误地跳转。
另外 7 个条件跳转(JEQ、JGE、JSET、JNE、JSGE、JLT、JSLT)的操作数正确。
3. ST/STX/LDX 中 emit_addi 使用 i16 offset,超出 12 位时静默截断
emit_st、emit_stx、emit_ldx 中通过 emit_addi(buf, RV_T1, base, insn.off as i32) 计算地址,但 RV ADDI 立即数仅 12 位有符号(±2048)。eBPF 的 off 字段为 i16(±32768),超出范围时 ADDI 会产生错误的有效地址。虽然常用场景中 offset 很小(BPF 栈空间 ≤ 512),但 insn_size 中已有 off_size 处理大 offset(预留 8 字节),说明代码预期支持大 offset——只是 emit 路径未实现对应的偏移加载。建议将 offset 加载改为与 emit_st 中立即数加载一致的 load_imm32 + add 模式,或至少在超出 12 位范围时显式报错而非静默截断。
🟡 建议改进
4. 缺少 JIT 编译结果的单元测试
| 当前无任何测试验证 JIT 生成的 RISC-V 指令序列正确性。建议添加 golden-match 单元测试,对每条指令翻译做简单验证。
5. emit_load_imm32 中 hi20 计算使用 +2048 而非 +0x800
| emit_load_imm64 中 32 位分支使用 +0x800(标准 RISC-V 惯例),但 emit_load_imm32 中使用 +2048。这两者等价(2048 = 0x800),但建议统一为 +0x800 提升可读性。
✅ 已修复的旧问题
- emit_call 寄存器 shuffle:r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅
- emit_load_imm64:使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列,lo12 使用
<<52>>52提取 ✅ - PC 相对跳转:AUIPC+load_imm64+ADD+JALR 替代绝对地址 ✅
- BPF_DIV 除零:dst 在分支后清零,正常除法保留原值 ✅
- JitBuffer Send/Sync 实现 ✅
- 模块重组到 ebpf/ebpf_jit/ 子目录 ✅
- rv_rw 辅助函数 ✅
- BPF_LSH/RSH/ARSH 寄存器移位 ANDI 移到移位之前 ✅
- ST/STX base 寄存器修复(从 RV_S5 改为 bpf_to_rv(dst)) ✅
- 条件跳转动态回填偏移(替代硬编码 28 字节) ✅
📝 其他观察
- Prologue/Epilogue 设计正确:callee-saved 保存/恢复、BPF 栈帧分配
JitBuffer::finalize()的 aarch64 dcache clean + icache flush 实现正确cargo check和cargo clippy --features ebpf均通过 ✅
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 的前 6 个 commit 包含了 #888 的全部内容。关系:partial-overlap,#888 是 #891 的前置依赖。建议先合并 #888,然后 rebase 本 PR。
- 未发现其他 eBPF JIT 相关的 open PR。
Powered by glm-5.1
Powered by deepseek-v4-pro
There was a problem hiding this comment.
PR #891 第八轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前七轮 review 基础上持续修复。本次审查基于当前 HEAD(9d638ccf6)。经过前几轮的迭代,许多之前的问题已修复:emit_load_imm64 64 位常量加载、emit_call 寄存器 shuffle、BPF_DIV 除零保护、BPF_MOD 除零(利用 RV REMU 原生行为)、动态分支偏移修补、rv_rw 辅助函数、模块重组、Send/Sync 等。本轮审查发现了 1 个新的阻塞性 bug 和 2 个设计建议。
🔴 阻塞性问题
1. 所有条件跳转的 AUIPC 偏移计算有 +4 字节偏差
emit_jmp 中条件跳转使用以下模式:
let branch_off = offsets[target_pc] - buf.offset(); // ← 在 BNE 之前计算
emit_bne(buf, dst, src, 0); // 4 字节
emit_auipc(buf, RV_T6, 0); // AUIPC 位于 buf.ptr + old_offset + 4
emit_load_imm64(buf, RV_T1, branch_off);
emit_add(buf, RV_T6, RV_T6, RV_T1);
emit_jalr(buf, RV_ZERO, RV_T6, 0);AUIPC 将当前 PC 存入 T6:T6 = buf.ptr + start + 4(start 是 BNE 的偏移)。
branch_off = offsets[target_pc] - start(以 BNE 之前为基准)。
最终跳转目标:T6 + T1 = (buf.ptr + start + 4) + (offsets[target_pc] - start) = buf.ptr + offsets[target_pc] + 4。
结果:所有条件跳转(JEQ/JGT/JGE/JSET/JNE/JSGT/JSGE/JLT/JLE/JSLT/JSLE)都跳转到目标指令之后 4 字节的位置,即跳过了目标指令的第一条。
修复方案:计算 branch_off 时以 AUIPC 位置为基准(buf.offset() + 4):
let auipc_pos = buf.offset() as isize + 4;
let branch_off = (offsets[target_pc] as isize - auipc_pos) as i32;注:BPF_JA 不受影响,因为它直接在 AUIPC 位置计算偏移,没有前面的 BNE/BEQ。
🟡 建议改进
2. 两遍编译的偏移估算不一致
insn_size 返回保守估计值(如 BPF_JA 固定 36 字节),但 pass2 中 emit_load_imm64 的实际大小可能是 4、8 或 24 字节,导致实际指令位置与 pass1 计算的 offsets[i] 不同。当后续指令的 pass2 实际偏移与 pass1 估算不一致时,所有以 offsets[target_pc] 为目标的跳转都可能偏移。当前代码能够工作是因为估计值始终 ≥ 实际值(保守估算),但累积误差会影响跳转精度。建议:
- 使
insn_size返回精确值,或 - 在 pass2 中动态重新计算 offsets
3. 缺少 JIT 编译结果的单元测试
当前没有任何测试验证 JIT 生成的 RISC-V 指令序列正确性。建议添加 golden-match 单元测试或集成测试,对每个指令翻译结果做验证。
📝 其他观察
- PR #891 包含了 PR #888(同作者 CN-TangLin)的全部 6 个 commits。建议先合并 #888,然后本 PR rebase 到 dev 分支。状态:#888 仍为 open,尚未合并。
emit_load_imm64当前实现正确 ✅emit_call寄存器 shuffle 正确 ✅- BPF_DIV 除零保护正确 ✅
- BPF_MOD 利用 RV REMU 原生行为正确处理除零 ✅
- Prologue/Epilogue 设计正确 ✅
JitBuffer的Send/Sync和Debug已正确实现 ✅JitBuffer::finalize()的架构相关缓存刷新正确 ✅- 模块重组清晰 ✅
验证状态
- ✅
cargo fmt --check通过 - ✅
cargo xtask clippy --package starry-kernel:14/14 feature 配置全部通过 ⚠️ 所有 GitHub Actions CI 检查状态为skipped(PR 来自 fork,可能需要手动批准 CI 运行)⚠️ 缺少 JIT 编译结果单元测试⚠️ 条件跳转 +4 偏移会导致任何包含条件分支的 eBPF 程序在 JIT 模式下跳转到错误位置
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 的前 6 个 commit 包含 #888 的全部内容。
- 未发现其他 eBPF JIT 相关的 open PR
总结
PR 在七轮 review 迭代后进展很大,大部分代码生成 bug 已修复。本轮发现的唯一阻塞性问题是条件跳转的 AUIPC 偏移计算偏差 +4 字节,会影响所有 11 种条件跳转指令的跳转目标。修复方法也很简单——将 branch_off 的计算基准从 BNE 之前改为 AUIPC 之后(+4 字节)。建议修复此问题后即可通过。需补充针对条件跳转的单元测试以验证修复效果。
Powered by deepseek-v4-pro
|
@CN-TangLin 如果这项工作还未完成,请关闭此项PR,并在完成后重新开启或提交。 |
There was a problem hiding this comment.
PR #891 第八轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前七轮 review 基础上持续修复。本次基于 HEAD 5bbb2eb3 进行独立代码审查,发现了 5 个新的阻塞性 bug(与之前 review 不同的问题点)。
🔴 阻塞性问题
1. Prologue 未保存 RA 寄存器——Helper 调用后返回到错误地址
emit_prologue 保存了 S1-S5 但未保存 RA(x1)。emit_call 中 JALR RA, T1, 0 会覆盖 RA 为 JIT buffer 内的返回地址。当 eBPF 程序包含任何 helper 调用后执行 BPF_EXIT 时,epilogue 的 JALR x0, RA, 0 会返回到 JIT buffer 内部而非原始调用者,导致无限循环或崩溃。
FRAME_SIZE 中有额外的 8 字节空间(+ 8),看起来是为 RA 预留的,但实际未使用。修复:在 prologue 添加 emit_sd(buf, RV_RA, RV_SP, BPF_STACK_SIZE + 40) 并在 epilogue 添加 emit_ld(buf, RV_RA, RV_SP, BPF_STACK_SIZE + 40)。
2. 条件跳转 AUIPC 偏移 -4 错误——所有条件分支跳转到目标前 4 字节
条件跳转中 branch_off 在 emit_auipc 之后计算:branch_off = offsets[target] - buf.offset()。但 AUIPC 在位置 P 加载 T6 = PC = buf_ptr + P,而 buf.offset() 此时已变为 P+4。导致 T6 + branch_off = buf_ptr + target - 4,比实际目标少 4 字节。
注意:无条件 JA 跳转中 off 是在 emit_auipc 之前计算的,因此 JA 是正确的。只有条件跳转有此 bug。
修复:在 emit_auipc 之前记录位置,或在 branch_off 计算中加 +4 补偿。
3. emit_load_imm64 低32位 ADDIW 符号扩展破坏高32位
完整 64 位加载路径中,低 32 位通过 emit_addiw(T1, T1, lo32_lo12) 构造。ADDIW 将结果符号扩展到 64 位——当 lo32 为负(bit31=1)时,T1 高 32 位全为 1。随后的 ADD rd, rd, T1 会因进位破坏 rd 中已正确放置的高 32 位。
例如 val=0x00000001_80000000:rd=1<<32=0x100000000,T1=sign_ext(0x80000000)=0xFFFFFFFF80000000,相加得到 0x80000000(错误)。
4. BPF 栈区域与 callee-saved 保存区重叠
r10(S5) = SP + FRAME_SIZE = SP + 560。BPF 栈通过 [r10-512, r10) 访问,即 [SP+48, SP+560)。但 callee-saved 寄存器保存在 [SP+512, SP+552),完全落在 BPF 栈区域内。eBPF 程序写入栈偏移 -48 到 -8 会覆盖保存的 S1-S5,导致 epilogue 恢复错误值。
修复:调整布局,将 callee-saved 放在 BPF 栈之下(低地址),或将 BPF 栈放在 callee-saved 之上。
5. insn_size 与实际 emit 大小不一致——offset 表错误导致所有分支目标偏移
两遍编译要求 pass1 的 insn_size 严格等于 pass2 的实际 emit 字节数。但当前:
LD_IMM64:insn_size 固定返回 24,但 emit_load_imm64 对小值只 emit 4-8 字节- 条件跳转:insn_size 假设 branch_off 需要 8 字节加载,但实际可能是 4 或 24 字节
- 这导致 offsets[] 表中的偏移比实际位置大,使后续所有分支跳转到错误位置
修复方案:让 insn_size 总是返回 emit 的最大可能大小,并确保 emit 始终输出该大小的代码(用 NOP 填充多余空间);或改为三遍编译。
📝 其他观察
- emit_call 寄存器 shuffle 已正确(r1→A0, r2→A1, ..., r5→A4)✅
- BPF_DIV 除零保护逻辑已修正 ✅
- BPF_MOD 依赖 RISC-V REMU 的除零行为(返回被除数),符合 eBPF 规范 ✅
- JitBuffer Send/Sync 已实现 ✅
- rv_rw 辅助函数可读性好 ✅
- 缺少 JIT 编译结果的单元测试
- 与 PR #888 存在代码重叠,建议先合并 #888 再 rebase
- @Godones 已建议关闭此 PR 待完成后重新提交
建议
考虑到已有 8 轮 review 仍持续发现新的代码生成 bug,强烈建议:
- 将 riscv64 后端提交到 rbpf 上游仓库,利用其数百个测试用例验证
- 添加针对 JIT 编译输出的 golden-match 单元测试
- 在 QEMU 上实际运行 eBPF 程序验证
- 重新开启 PR 时确保上述测试全部通过
Powered by glm-5.1
| fn emit_alu(buf: &mut JitBuffer, insn: &BpfInsn, is_64: bool) { | ||
| let dst = bpf_to_rv(insn.dst_reg()); | ||
| let use_imm = (insn.code & BPF_X) == 0; | ||
| let src = if use_imm { |
There was a problem hiding this comment.
[Bug #1] RA 未保存
Prologue 保存了 S1-S5 但未保存 RA。FRAME_SIZE 中有 + 8 的空间(看起来为 RA 预留),但实际未使用。
emit_call 中 JALR RA, T1, 0 会覆盖 RA。之后 epilogue 的 emit_ret() (即 JALR x0, RA, 0) 会返回到 JIT buffer 内部而非原始调用者。
修复:
// prologue 中添加:
emit_sd(buf, RV_RA, RV_SP, (BPF_STACK_SIZE + 40) as i32);
// epilogue 中添加(在 restore S1-S5 之前):
emit_ld(buf, RV_RA, RV_SP, (BPF_STACK_SIZE + 40) as i32);| if use_imm { | ||
| if is_64 { | ||
| emit_load_imm64(buf, RV_T1, insn.imm as u64); | ||
| } else { |
There was a problem hiding this comment.
[Bug #2] AUIPC 偏移 -4 错误
branch_off = offsets[target_pc] - buf.offset() 是在 emit_auipc 之后计算的。此时 buf.offset() = AUIPC 位置 + 4。但 AUIPC 加载的是自身位置 P 处的 PC,所以:
T6 = buf_ptr + P
branch_off = target - (P + 4)
T6 + branch_off = buf_ptr + target - 4 ← 差 4 字节!
对比无条件 JA 跳转中 off 是在 emit_auipc 之前计算的,JA 是正确的。
所有条件跳转(JEQ/JGT/JGE/JSET/JNE/JSGT/JSGE/JLT/JLE/JSLT/JSLE)都有此 bug。
修复:在 emit_auipc 之前记录 let auipc_pos = buf.offset(),然后用 offsets[target_pc] - auipc_pos 计算偏移。
| fn emit_load_imm32(buf: &mut JitBuffer, rd: u32, val: i32) { | ||
| let needs_upper = (val as i32) < -2048 || (val as i32) >= 2048; | ||
| if !needs_upper { | ||
| emit_addi(buf, rd, RV_ZERO, val); |
There was a problem hiding this comment.
[Bug #3] ADDIW 符号扩展破坏高32位
当 lo32 为负(bit31=1)时,emit_addiw(T1, T1, lo32_lo12) 将结果符号扩展到 64 位,T1 高 32 位全为 1。随后的 emit_add(rd, rd, T1) 因进位破坏 rd 中已正确放置的 hi32。
示例:val = 0x00000001_80000000
- rd = hi32 << 32 = 0x00000001_00000000
- T1 = sign_ext(0x80000000) = 0xFFFFFFFF_80000000
- rd + T1 = 0x00000000_80000000 ≠ 0x00000001_80000000
参考 Linux kernel arch/riscv/net/bpf_jit_comp64.c 的 emit_imm 函数,使用调整后的 upper32 值来吸收低位进位。
| emit_ld(buf, RV_S2, RV_SP, (BPF_STACK_SIZE + 8) as i32); | ||
| emit_ld(buf, RV_S3, RV_SP, (BPF_STACK_SIZE + 16) as i32); | ||
| emit_ld(buf, RV_S4, RV_SP, (BPF_STACK_SIZE + 24) as i32); | ||
| emit_ld(buf, RV_S5, RV_SP, (BPF_STACK_SIZE + 32) as i32); |
There was a problem hiding this comment.
[Bug #4] BPF 栈与 callee-saved 重叠
S5 (r10/frame pointer) = SP + FRAME_SIZE = SP + 560。
BPF 栈范围:[S5-512, S5) = [SP+48, SP+560)。
但 callee-saved 保存在 [SP+512, SP+552),完全在 BPF 栈区域内!
eBPF 程序写入栈偏移 -48 到 -8 会覆盖保存的 S1-S5,导致 epilogue 恢复错误值。
修复建议:将 callee-saved 放在栈底部:
[SP, SP+40): callee-saved S1-S5 + RA
[SP+48, SP+560): BPF 栈 (512 bytes)
S5 = SP + 560
这样 [S5-512, S5) = [SP+48, SP+560) 是纯 BPF 栈区域。
| } | ||
| let off = insn.off as i32; | ||
| let base = bpf_to_rv(insn.dst_reg()); | ||
| emit_add_offset(buf, RV_T1, base, off); |
There was a problem hiding this comment.
[Bug #5] insn_size 与实际 emit 大小不匹配
两遍编译策略要求 insn_size 严格等于 pass2 实际 emit 的字节数。但:
LD_IMM64固定返回 24,但emit_load_imm64对小值只 emit 4 或 8 字节- 条件跳转固定 40+ 字节,但 branch_off 的 load_imm64 大小是可变的(4/8/24)
当 insn_size 过大时,offsets[] 中后续指令的偏移会比实际位置大,导致所有分支跳转到错误位置。
建议采用固定最坏情况策略:insn_size 返回最大值,emit 时用 NOP 填充。或改为三遍编译。
f4f3a9b to
33ef522
Compare
There was a problem hiding this comment.
PR #891 第八轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢在前七轮 review 基础上持续修复,许多之前的问题已经解决。本次审查基于当前 HEAD(33ef522f9)。
已修复的旧问题 ✅
emit_call寄存器 shuffle:正确映射 r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 ✅emit_load_imm64:使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 ✅- 跳转使用 AUIPC PC 相对寻址 ✅
- BPF_DIV 除零保护:BEQ+DIVU+JAL+ADDI 模式正确 ✅
- BPF_MOD:REMU 在 RISC-V 中 divisor=0 时返回被除数,符合 eBPF 语义 ✅
JitBufferSend/Sync ✅rv_rw辅助函数 ✅- 模块重组到
ebpf/ebpf_jit/子目录 ✅ - BPF_LSH 寄存器移位 ANDI 位置修正 ✅
- BPF_EXIT 由 JitCompiler::compile() 直接调用 emit_epilogue 处理 ✅
🔴 阻塞性问题
1. 两遍编译中 insn_size 系统性偏大,导致 offset 漂移和跳转目标错误
两遍编译的核心约束是:每条 BPF 指令的实际 emit 字节数必须精确等于 insn_size() 返回值,或者实际 < 估计时用 NOP 填充到估计值。当前代码中只有 BPF_LD|BPF_IMM|BPF_DW 有 NOP 填充,其余指令均无填充。关键不一致:
- 条件跳转(JEQ/JGT/JNE 等 11 种):
insn_size用固定常量 40 估算分支块,但实际分支块 = BNE(4)+AUIPC(4)+load_imm64(4或8)+ADD(4)+JALR(4) = 2024 字节。每条条件跳转**多估 1620 字节**。 - BPF_CALL:insn_size=56,实际=6×MV(24)+load_imm64(4
24)+JALR(4)=3252 字节,多估 4~24 字节。 - BPF_JA:insn_size=36,实际=AUIPC(4)+load_imm64(4
24)+ADD(4)+JALR(4)=1636 字节,最多多估 20 字节。
后果:pass 1 构建的 offsets[] 表中,每条过估指令都会使后续所有 offset 偏大。pass 2 中所有使用 offsets[target_pc] 的跳转(BPF_JA、所有条件跳转的 "taken" 路径)都会跳到实际代码之后的位置,执行错误的指令。
注意:条件跳转中 BNE 回填的 "not-taken" 路径使用 end - start(精确值),是正确的。问题仅在于使用 offsets[] 的 "taken" 路径。
建议修复方向:
- 方案 A:使所有 emit 函数在输出后 NOP 填充到 insn_size 指定的大小(最简单,但浪费空间)
- 方案 B:对分支块内的
emit_load_imm64也使用固定最坏情况(如始终用 24 字节的完整序列)+ NOP 填充,确保 insn_size 精确 - 方案 C:多轮迭代直到 offsets 收敛(类似 Linux 内核 riscv64 JIT 的做法)
📝 其他观察
- Prologue/Epilogue 设计正确
JitBuffer::finalize()的架构相关缓存刷新实现正确- 缺少 JIT 编译结果的单元测试来验证生成的 RISC-V 指令序列
- 无实际 QEMU 运行验证
Duplicate/Overlap 分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础(解释器+验证器+map+helper+perf_event)。本 PR 前数个 commit 包含 #888 的全部内容。
- 未发现其他 eBPF JIT 相关的 open PR
验证状态
- ✅
cargo fmt --check通过 - ✅
cargo xtask clippy --package starry-kernel:PR 声明全部通过 ⚠️ 缺少 JIT 编译结果单元测试⚠️ 无 QEMU 运行验证
Powered by glm-5.1
| 4 | ||
| } else { | ||
| 8 | ||
| } |
There was a problem hiding this comment.
🔴 分支块大小常量 40 系统性偏大
实际分支块大小 = BNE(4) + AUIPC(4) + emit_load_imm64(4或8) + ADD(4) + JALR(4) = 20~24 字节。
因为 branch_off 是 i32 范围内的值(offsets[target_pc] - auipc_pos 的差值),emit_load_imm64 对 i32 值只会产生 4 或 8 字节的输出(走 ADDI 或 LUI+ADDI 分支),不会走 24 字节的完整 64 位路径。
这里用固定 40 导致每条条件跳转多估 16~20 字节。多估的量会累积到后续所有 offsets[] 条目中,导致所有使用 offsets 的 "taken" 跳转路径指向错误位置。
建议:将 40 改为 20 + 最大 load_imm64 字节数(对 i32 范围偏移,最大 8)= 28,并在 emit 后 NOP 填充到 insn_size 指定大小。或者统一使用固定大小的 load_imm64(如始终用 LUI+ADDI=8 字节),使实际 emit 精确匹配 insn_size。
| emit_bne(buf, dst, src, 0); | ||
| let auipc_pos = buf.offset(); | ||
| emit_auipc(buf, RV_T6, 0); | ||
| let branch_off = (offsets[target_pc] as isize - auipc_pos as isize) as i32; |
There was a problem hiding this comment.
此处 branch_off 来自 offsets[target_pc],而这个表在 pass 1 中基于过大的 insn_size 构建。虽然 BNE 回填的 "not-taken" 路径(跳过分支块)使用精确的 end - start 是正确的,但这里的 "taken" 路径(跳转到目标 BPF 指令)使用了可能有漂移的 offsets[target_pc]。
当前面有指令的实际 emit 小于 insn_size 估计时,offsets[target_pc] 会偏大,导致 AUIPC+load_imm64+ADD+JALR 计算出的跳转目标在实际代码之后,执行到错误的指令。
| if op == BPF_EXIT { | ||
| 32 | ||
| } else if op == 0x80 { | ||
| 24 + 24 + 4 + 4 |
There was a problem hiding this comment.
BPF_CALL 的 insn_size 估算为 56 字节,但 emit_call 实际输出 = 6×MV(24) + emit_load_imm64(424) + JALR(4) = 3252 字节。即使 helper 函数地址需要完整的 24 字节 64 位加载(52 字节实际),仍多估 4 字节;若地址在低 2KB 范围内(32 字节实际),则多估 24 字节。
同样会导致后续 offset 漂移。建议调整 insn_size 使其更接近实际,或在 emit_call 后 NOP 填充到指定大小。
…ndant ADDIW 1. insn_size: add page-aligned imm check (imm & 0xFFF == 0 → size 4) for JMP and ST paths, matching the ALU path optimization. 2. emit_load_imm64 64-bit path: skip ADDIW when lower_lo12 == 0, eliminating redundant NOP instruction.
There was a problem hiding this comment.
PR #891 第十一轮审查:eBPF JIT 编译器框架 + RISC-V 64 后端
本次审查基于当前 HEAD 4167a138d。经过 10 轮 review 和持续修复,所有之前发现的阻塞性问题均已修复,代码质量良好。
变更概览
本 PR 为 StarryOS 添加了 eBPF JIT 编译框架(ebpf/ebpf_jit/mod.rs)和完整的 RISC-V 64 后端(jit_riscv64.rs,~1500 行),包含 x86_64/aarch64 stub 后端。同时增强了 ebpf.rs 中的 map 类型(RingBuffer、ProgArray、StackTrace、PerCpuArray)和 helper 函数(tail_call、ringbuf 等)。
已修复的阻塞性问题 ✅
逐项验证当前代码:
-
emit_call寄存器 shuffle ✅ — 当前实现:T2←A5; A0←A1; A1←A2; A2←A3; A3←A4; A4←T2,正确映射 BPF r1→A0, r2→A1, r3→A2, r4→A3, r5→A4 -
emit_load_imm6464位立即数加载 ✅ — 使用无符号算术(val.wrapping_sub(lo12).wrapping_add(0x800)) >> 12+& 0xFFFFF掩码,三路径(小值/32位/64位)正确。64位路径:LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列 -
BPF_DIV 除零保护 ✅ —
BEQ src,0,+12; DIVU dst,dst,src; JAL +8; ADDI dst,x0,0— src≠0 时正常除法,src==0 时 dst=0(符合 eBPF 规范) -
BPF_MOD 除零保护 ✅ —
BEQ src,0,+skip; REMU dst,dst,src— src==0 时跳过 REMU,dst 保持原值(符合 eBPF 规范) -
PC 相对跳转 ✅ — 所有条件跳转和无条件跳转均使用 AUIPC + load_imm64_padded + ADD + JALR 模式
-
insn_size 精确估算 ✅ — 包含页对齐立即数优化(lo12==0 时 load_size=4)、BPF_DIV=16、BPF_MOD=8、条件跳转=40+jset_extra
-
JitBufferSend/Sync ✅ —unsafe impl Send/Sync for JitBuffer -
模块结构 ✅ —
ebpf/ebpf_jit/子目录,re-export 清晰
验证
- ✅
cargo fmt --check通过 - ✅ 28 个单元测试(8 个架构无关 hi20/lo12 + 20 个 riscv64 golden-match)
- ✅
insn_size与实际 emit 长度精确匹配(DIV/MOD/ALU32/MOV32/ARSH/ST/JMP 等) ⚠️ CI 仅有一个「Detect changed paths」check 排队中,暂无失败⚠️ 无 QEMU 运行时验证(JIT 当前不直接触发,sys_bpf 路由到 eBPF 模块)⚠️ BPF_END(字节序转换)未实现,已添加 warn 日志
Duplicate/Overlap 分析
- PR #888(同作者):已关闭,未合并。#848 已包含其功能。本 PR 已 rebase 到 dev,仅保留 JIT commit,无重叠
- PR #892(x86_64 JIT)、#893(aarch64 JIT):下游依赖本 PR,无冲突
- 未发现其他 eBPF JIT 相关 open PR
关于 Godones 的建议
@Godones 建议将 JIT 后端贡献到 rbpf 上游。@CN-TangLin 已同意后续执行。当前 StarryOS 中的 JIT 实现仍有必要作为内核集成的基础设施,上游贡献可以并行进行。不构成阻塞。
关于 apps 测试
本 PR 不包含 apps/starry 下的新增测试或 test-suit 用例。变更为 eBPF JIT 基础设施代码,不涉及用户态应用行为。因此无需 QEMU apps 运行验证。
总结
经过 10+ 轮 review 和修复,当前代码的所有关键代码生成 bug 已消除。JIT 后端的指令翻译(ALU/JMP/MEM/CALL/EXIT)、寄存器映射、64位立即数加载、除零保护、PC 相对跳转、insn_size 估算均正确。Prologue/Epilogue 设计合理。代码组织清晰。建议合入。
Powered by mimo-v2.5-pro
…4 fix) Project uses #[macro_use] extern crate ax_log, not the log crate directly.
|
@luodeb 烦请Debin老师审校和提出建议 |
There was a problem hiding this comment.
PR #891 审查通过:eBPF JIT 编译器框架 + RISC-V 64 后端
感谢经过 10 轮 review 迭代后的高质量代码。本 PR 为 StarryOS 添加了完整的 eBPF JIT 编译框架和 riscv64 后端,整体架构设计合理,两遍编译策略与 Linux 内核一致。
已验证的关键修复 ✅
经过逐行审查当前 HEAD(5384656e),确认前 10 轮 review 提出的所有阻塞性问题均已修复:
emit_call寄存器 shuffle ✅ — 正确映射 r1→A0, r2→A1, r3→A2, r4→A3, r5→A4(T2 临时保存 A5)emit_load_imm6464 位立即数加载 ✅ — 使用 LUI+ADDIW+SLLI+LUI+ADDIW+ADD 标准序列,unsigned wrapping_sub + 0x800 偏移处理 hi20 正确- PC 相对跳转 ✅ — 使用 AUIPC+load_imm64_padded+ADD+JALR 模式,branch_off 在 AUIPC 之后计算
- BPF_DIV 除零保护 ✅ — BEQ skip + DIVU + JAL skip_zero + ADDI dst,x0,0 正确模式
- BPF_MOD 除零保护 ✅ — BEQ skip + REMU,除零时保持 dst 不变(REMU 硬件行为)
insn_size精确估算 ✅ — DIV=16, MOD=8, 条件跳转使用动态回填- JIT buffer 安全性 ✅ —
unsafe impl Send/Sync、page 对齐、finalize 缓存刷新 - BPF_EXIT 处理 ✅ — compile() 中直接调用 emit_epilogue,无死代码
- Callee-saved 一致性 ✅ — prologue 保存 RA/S1-S5,epilogue 恢复,S5 帧指针设置正确
验证结果
- ✅
cargo xtask clippy --package starry-kernel:13/13 feature 配置全部通过 - ✅
cargo fmt --check:通过 - ✅ 28 个单元测试(8 个架构无关 hi20/lo12 测试 + 20 个 riscv64 目标测试)覆盖 emit_load_imm64 边界和 insn_size golden-match
⚠️ 无 apps 测试,无需 QEMU 验证(本 PR 仅添加 JIT 基础设施)
重复/重叠分析
- PR #888(同作者 CN-TangLin):eBPF 子系统基础,已被关闭(#848 已合并到 dev),不再构成重叠。
- PR #892(同作者):x86_64 JIT 后端,以 #891 为前置依赖。互补关系,无冲突。
- PR #893(同作者):aarch64 JIT 后端,以 #891 和 #892 为前置依赖。互补关系,无冲突。
- dev 分支上 #847(kprobe)、#848(eBPF 子系统)、#849(LKM)、#874(eBPF 测试)均已合并,本 PR 的前置依赖均满足。
- 未发现其他 eBPF JIT 相关的 open PR。
实现逻辑
- 两遍编译策略:Pass 1 计算 insn_size 建立 PC→offset 映射,Pass 2 生成机器码
- 编译失败安全回退解释器(try_jit_compile 返回 None)
- BPF 栈帧:512 字节 BPF 栈 + 48 字节 callee-saved 区域
- 页对齐立即数优化:lo12==0 时 load_size=4(仅 LUI)
- Stub 后端(x86_64/aarch64)确保跨架构编译通过
已知局限
sys_bpf在 syscall 中路由到sys_dummy_fd,JIT 代码当前不会被实际调用(预集成,需后续 syscall 路由接入)- BPF_END(字节序转换)未实现(已添加 warn 日志)
- x86_64/aarch64 stub 后端待后续 PR 实现
Powered by mai-reviewer
Powered by mimo-v2.5-pro
|
@CN-TangLin 同学,建议fork rbpf库并将jit支持放到rbpf库中,由于这些功能通常是系统无关的,直接放到Starry中并不合适。即使不能马上推到上游rbpf中,也可以使用git 依赖你修改的rbpf。 |
|
@Godones @luodeb 感谢助教老师的指导和建议! 我已经按照我们之前讨论的方案,完成了所有 JIT 相关的重构和迁移工作,具体进展如下:
麻烦老师们有空时帮忙 re-review 一下最新的代码,非常感谢! |
|
根据助教建议,本 PR 包含了过多的历史 JIT 实现细节(10多轮的 commit),且测试用例迁移有误。本 PR 现作废并关闭,已开启全新的精简 PR #1035 来替代此工作。 |
将 rcore-os#850 的 eBPF 运行时重建到当前 dev 之上,替换 rcore-os#848 合入的单文件实现 (`ebpf.rs` ~1503 行 + `perf_event.rs` ~509 行),改为 `ebpf/` + `perf/` 模块树,复用 `kbpf-basic` + `rbpf` 上游 crate,而非手写 VM/verifier/map。 经 reviewer 评审与 maintainer、rcore-os#848 作者 @CN-TangLin 同意后,按既定方针 重建(非简单 rebase——本分支原 base 落后 dev 49 个 commit,含 rcore-os#963 HAL 迁移 / rcore-os#849 LKM / 新增 syscall): - 原则冲突(以 sys_bpf 为本体,而非 LKM 实现 eBPF)以本实现为准; - 事件发掘统一走 perf 子系统:删除 rcore-os#848 在 `kprobe.rs` 注入的全局 `perf_event_trigger_by_type` 广播,改为每个 perf fd 直接挂 `OwnedEbpfVm` 回调(`register_event_callback`); - 其余重叠按 rcore-os#673 > 本实现 > rcore-os#847: - `tracepoint/mod.rs` 以 rcore-os#673 基座为底,仅追加 `lookup_ext_tracepoint` / `find_ext_tracepoint_by_name`; - `kprobe.rs` 在 rcore-os#847 基座(保留其 ax_runtime::hal API、可执行内存映射、 loongarch64 显式分支)上追加 `KernelRawMutex`、register/unregister 公开 API、`KPROBE_POINT_LIST`、类型别名,供 perf 子模块使用。 其他: - `kallsyms.rs` 精简至实际使用面(`kallsyms_init` + `kallsyms_lookup_name`), `/proc/kallsyms` 仍由 dev 的 `ksym` crate 独立提供;符号数据喂入留作后续。 - `ebpf`/`perf`/`kallsyms` 模块沿用 dev 的 `ebpf` feature(默认开启)门控; `kbpf-basic`/`rbpf` 作为该 feature 的可选依赖。 - 工作区临时 `[patch.crates-io]` 指向 printf-compat nightly-fix-0.3 分支 (kbpf-basic 0.5 间接锁定的 printf-compat 0.3.1 在当前 nightly 不可编译)。 验证:`cargo fmt --all -- --check` 通过;x86_64 / aarch64 / riscv64 / loongarch64 四架构 `cargo xtask starry build` 全部通过(本仓代码 0 warning)。 下游 rcore-os#849 (LKM) / rcore-os#874 (用户态测试) / rcore-os#891 (JIT) 原建立在 rcore-os#848 的 API 上, 需 retarget 到本实现的 `ebpf/` + `perf/` 接口。
…850) * feat(starry-kernel): port modular eBPF runtime over perf subsystem 将 #850 的 eBPF 运行时重建到当前 dev 之上,替换 #848 合入的单文件实现 (`ebpf.rs` ~1503 行 + `perf_event.rs` ~509 行),改为 `ebpf/` + `perf/` 模块树,复用 `kbpf-basic` + `rbpf` 上游 crate,而非手写 VM/verifier/map。 经 reviewer 评审与 maintainer、#848 作者 @CN-TangLin 同意后,按既定方针 重建(非简单 rebase——本分支原 base 落后 dev 49 个 commit,含 #963 HAL 迁移 / #849 LKM / 新增 syscall): - 原则冲突(以 sys_bpf 为本体,而非 LKM 实现 eBPF)以本实现为准; - 事件发掘统一走 perf 子系统:删除 #848 在 `kprobe.rs` 注入的全局 `perf_event_trigger_by_type` 广播,改为每个 perf fd 直接挂 `OwnedEbpfVm` 回调(`register_event_callback`); - 其余重叠按 #673 > 本实现 > #847: - `tracepoint/mod.rs` 以 #673 基座为底,仅追加 `lookup_ext_tracepoint` / `find_ext_tracepoint_by_name`; - `kprobe.rs` 在 #847 基座(保留其 ax_runtime::hal API、可执行内存映射、 loongarch64 显式分支)上追加 `KernelRawMutex`、register/unregister 公开 API、`KPROBE_POINT_LIST`、类型别名,供 perf 子模块使用。 其他: - `kallsyms.rs` 精简至实际使用面(`kallsyms_init` + `kallsyms_lookup_name`), `/proc/kallsyms` 仍由 dev 的 `ksym` crate 独立提供;符号数据喂入留作后续。 - `ebpf`/`perf`/`kallsyms` 模块沿用 dev 的 `ebpf` feature(默认开启)门控; `kbpf-basic`/`rbpf` 作为该 feature 的可选依赖。 - 工作区临时 `[patch.crates-io]` 指向 printf-compat nightly-fix-0.3 分支 (kbpf-basic 0.5 间接锁定的 printf-compat 0.3.1 在当前 nightly 不可编译)。 验证:`cargo fmt --all -- --check` 通过;x86_64 / aarch64 / riscv64 / loongarch64 四架构 `cargo xtask starry build` 全部通过(本仓代码 0 warning)。 下游 #849 (LKM) / #874 (用户态测试) / #891 (JIT) 原建立在 #848 的 API 上, 需 retarget 到本实现的 `ebpf/` + `perf/` 接口。 * ci: retrigger after pre-existing axtask SMP flake 上次 CI 唯一真失败是 riscv64 test-rawmutex-handoff 触发 axtask 调度器断言 panic(os/arceos/modules/axtask/src/run_queue.rs:716,SMP 上下文切换的 Arc::strong_count 竞态),与本 PR 改动(ebpf/perf/kprobe)无关;x86_64 / loongarch64 同份内核代码均通过。aarch64 qemu 与 clippy 为 fail-fast 连带取消。 空提交重跑 CI 验证为偶发。 * refactor(starry-kernel): address eBPF runtime review feedback 按 @Godones review 意见收敛到内核既有抽象、去掉多余复杂度: - 复用既有设施:kprobe 符号解析改走真实 .kallsyms(pseudofs::proc::KALLSYMS), 删除空的 kallsyms stub 模块;alloc_page/free_page 复用 mm::aspace::backend 的 alloc_frame/dealloc_frame;sys_bpf 用 kbpf_basic::linux_bpf::bpf_cmd 取代手写常量。 - 去锁:rbpf execute_program 为 &self,OwnedEbpfVm 执行方法改 &self,移除 kprobe / raw_tracepoint / tracepoint 三处多余的 spin::Mutex。 - 修正:TracepointPerfEvent enable/disable 对每个 TraceEventFunc 调 set_perf_enable —— 否则 ktracepoint 因 perf_enabled()==false 永不触发回调。 - 组织:sys_perf_event_open 移入 crate::perf;移除 ebpf feature,默认编入。 - 安全:register_allowed_memory(0..u64::MAX) 处补 TODO。 本地验证:fmt / clippy(-D warnings, 12 配置) / x86_64 / loongarch64 build 均通过。 * ci: retrigger after pre-existing axvisor x86_64 smoke flake * build(starry-kernel): bump kbpf-basic to 0.5.7, drop git dependency kbpf-basic 0.5.7 已合并 printf-compat 0.4 迁移并改用 ax-errno 0.6, 因此移除临时的 printf-compat git patch。改用 path patch 把 kbpf-basic 依赖的 ax-errno 重定向到工作区自身的副本,使 BpfError 与内核 AxError 为同一类型,避免 ax-errno 重复编译。 * refactor(starry-kernel): address eBPF runtime review round 2 - ebpf: drop the bpf_to_ax_err table; kbpf-basic 0.5.7 now uses the in-tree ax-errno so BpfError = LinuxError and `?` auto-converts via AxError: From<LinuxError>. - ebpf: return InvalidInput (-EINVAL) for unknown/unsupported bpf(2) commands instead of Unsupported (-ENOSYS), matching Linux. - perf/ebpf: honour PERF_FLAG_FD_CLOEXEC in perf_event_open and create bpf map/prog/raw-tracepoint fds close-on-exec (Linux bpf fds are always O_CLOEXEC). - kprobe: replace the hand-written AtomicBool raw mutex (no preempt/IRQ disable, deadlock-prone on trap re-entry) with ax_kspin::RawSpinNoIrq. feat(kspin): add `lock_api` feature exposing BaseRawSpinLock<G> / RawSpinNoIrq, a NoPreemptIrqSave-backed lock_api::RawMutex. * fix(kspin): use Acquire load in raw spinlock spin-wait The inner spin-wait in `BaseRawSpinLock::lock` read `locked` with `Ordering::Relaxed`, which sync-lint flags (suspicious_relaxed_wait_condition and suspicious_relaxed_mixed_ordering). Match `BaseSpinLock::is_locked` and load with `Acquire` so all accesses to `locked` use consistent orderings.
概述
引入
rbpf作为 eBPF JIT 编译器的核心依赖,将 JIT 支持解耦出 StarryOS 内部,同时将相关的 eBPF 用户态测试程序迁移到apps/目录下。背景与重构说明
根据助教 (@Godones) 的建议,由于 JIT 编译器(RISC-V 64 / AArch64 / x86_64)是系统无关的,已将原先在 PR #891、#892、#893 中手写的全部 JIT 代码直接 fork 并合并到了上游
rbpf库中。StarryOS 改为通过 Git 依赖引入修改后的rbpf。rbpf库中统一处理。qmonnet/rbpf提交了 PR [Refactor] 移除 linkme 并使用 EII #147 以添加 RISC-V 64-bit JIT backend。StarryOS 端的变更内容
1. JIT 编译框架解耦
ebpf_jit目录)。rbpf_jit.rs桥接层,封装了rbpf::jit::JitMemory并处理了参数签名的映射。os/StarryOS/kernel/Cargo.toml增加对rbpf(fork 版本) 的 Git 依赖。2. 用户态测试程序迁移
test-ebpf-advanced,test-ebpf-attach,test-ebpf-basics) 从 CI 测试目录test-suit/starryos/normal/qemu-smp1/syscall迁移至apps/starry/ebpf。README.md说明其作为综合应用测试的用途。🚀 附录:已迁移至 rbpf 库的核心技术实现细节 (Historical Record)
(以下为原 PR 期间在 StarryOS 内部实现的 RISC-V 64 JIT 核心逻辑记录,现已原封不动迁移至上游
rbpf)RISC-V 64 后端核心实现
wrapping_sub+& 0xFFFFF),修复了有符号溢出导致 LUI 编码为 0 的 bug经过 10 轮 Review 的核心修复记录
emit_alu末尾统一处理)。emit_load_imm64LUI 立即数溢出 bug 修复(无符号算术 + 掩码 20 位 + ADDIW)。