feat(axbuild): enable x86 kvm acceleration#1221
Merged
Merged
Conversation
ZR233
marked this pull request as ready for review
June 11, 2026 08:04
Merged
Contributor
There was a problem hiding this comment.
评审总结
此 PR 为 axbuild 增加了 x86_64 KVM 加速的自动检测与注入能力,已在 dev 分支合入。以下为本次评审的分析结果。
改动分析
PR 修改了 2 个文件(scripts/axbuild/src/context/mod.rs 和 scripts/axbuild/src/test/qemu.rs),核心逻辑:
- KVM 可用性检测:通过尝试以读写方式打开
/dev/kvm判断宿主机是否支持 KVM(仅在 unix 平台有效,非 unix 平台直接返回 false)。 - 自动注入策略:当目标架构为 x86_64、宿主机
/dev/kvm可读写、且 QEMU 参数中尚未包含-accel时,自动追加-accel kvm。保留已有显式 accelerator 配置(如-accel tcg,thread=single)。 - 两个注入入口:
apply_dynamic_platform_qemu_boot:覆盖通过动态平台启动改写入口的场景(ArceOS/Starry/axvisor 测试)。AppContext::qemu:覆盖 Starry QEMU case 中不经过动态平台改写入口的场景。
- 可测试性设计:将 KVM 探测逻辑抽象为
FnOnce() -> bool参数,使单元测试可以通过注入闭包精确控制 KVM 可用/不可用状态,无需依赖实际/dev/kvm。
实现质量
- 代码风格与现有模式一致,函数命名清晰,职责分离合理。
- 将
cargo_target_is_x86_64从cargo_target_is_dynamic_x86_64中抽取为独立函数,便于复用,重构干净。 - 新增 5 个单元测试覆盖所有关键路径:KVM 可用、动态启动路径、显式 accelerator 保留、非 x86_64 跳过、KVM 不可用。
- 将现有测试升级为使用
apply_dynamic_platform_qemu_boot_with_kvm_probe(&mut qemu, &cargo, || false),保持原有行为不变。 - 无
[patch.crates-io]问题,无新增依赖。
验证结果
cargo fmt --check:✅ 通过cargo clippy --manifest-path scripts/axbuild/Cargo.toml --all-features -- -D warnings:✅ 通过cargo test --manifest-path scripts/axbuild/Cargo.toml --all-features:629 passed, 1 failed- 失败测试
build::tests::checked_in_build_configs_do_not_declare_default_dynamic_builds为 pre-existing 问题(由 base 分支提交ab011053b feat(axbuild): default dynamic platform builds引入),与本 PR 修改的文件无关。此失败不属于本 PR 引入的回归。
- 失败测试
CI 状态
远程 CI 所有 check runs 均显示 skipped。这是路径过滤的预期行为——scripts/axbuild/ 变更仅触发 clippy 和 format 检查(这些检查在 skipped 列表中),其余架构/平台测试被路径过滤跳过。由于 CI 没有本 PR 引起的失败,且所有与 PR 相关的工作流均已 skipped(非 failure/cancelled),CI 状态不构成阻塞因素。
重复/重叠分析
- 在 base 分支中搜索
kvm相关内容,确认此功能在 base 上不存在,为全新特性。 - 搜索相关 open PR,未发现重复或冲突的 PR。PR #1166(axvisor x86_64 guest)和 #1024(x86_64 dynamic platform)属于不同功能领域,无重叠。
潜在关注点(非阻塞)
-accel kvm 被追加到 QEMU 参数列表末尾。在绝大多数情况下 QEMU 对 -accel 的位置不敏感,但极少数场景下如果后续参数(如 -cpu host)依赖 KVM 已启用,可能需要调整参数顺序。当前仓库的 x86_64 配置中不存在此类依赖,因此无实际问题。
评审结论
代码实现正确、测试充分、无阻塞性问题。已合入 dev 分支,本评审记录为 post-merge 确认。
Powered by deepseek-v4-pro
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题
当前 x86_64 QEMU 测试默认不会根据宿主机能力自动启用 KVM。即使当前用户已经具备
/dev/kvm读写权限,也需要在各个 qemu 配置里显式维护-accel kvm,容易造成配置漂移。改动
/dev/kvm可读写、且 QEMU 参数中没有显式-accel时,自动追加-accel kvm。-accel tcg,thread=single,避免覆盖特殊测试行为。AppContext::qemu路径中也应用同一 KVM 注入逻辑,覆盖 Starry QEMU case 这类不会经过动态平台启动改写入口的场景。验证
cargo test -p axbuild --lib qemu -- --nocapturecargo fmtcargo xtask clippy --package axbuildcargo xtask starry test qemu --arch x86_64 -c qemu-smp1/system/bug-advisory-lock-dir,测试通过,并通过 strace 确认最终启动参数包含-accel kvm。