feat(axvisor): add PhytiumPi and ROC-RK3568 board tests#934
Conversation
![mai-team-app[bot]](https://avatars.githubusercontent.com/u/34859362?s=60&v=4){kind=small}
There was a problem hiding this comment.
审查总结
PR #934 为 Axvisor 增加了 PhytiumPi 和 ROC-RK3568-PC 两块板子的测试用例，并改进了 FDT CPU 映射逻辑。整体方向正确，但存在一个需要修复的错误处理回归问题。
变更概览
- 新增板级测试用例：
phytiumpi-linux和roc-rk3568-pc-linux，遵循现有test-suit/axvisor/normal布局。 - 板级驱动特性更新：PhytiumPi 使用
phytium-mci，ROC-RK3568-PC 使用rockchip-sdhci。 - CI 变更：简化 loongarch64 QEMU CI 为统一命令
cargo xtask axvisor test qemu --arch loongarch64，新增两块自托管板级 CI。 - FDT CPU 映射重构：分离了 host CPU node 选择（基于
cpu@Xunit address）与 guest-visible CPU ID（来自reg属性），增加了set_guest_phys_cpu_idssetter。 - riscv64 runtime FDT 修补：使 host FDT 访问变为可选，改善了对缺失 host FDT 场景的容错。
验证结果
cargo fmt --check：通过- 单元测试（
cpu_node_id、cpu_node_selection_uses_node_id_when_reg_differs）：新增测试覆盖了 CPU 节点选择的核心逻辑 - GitHub Actions CI：check suite
70603051358结论为 success
重复/冲突分析
未发现与 #934 功能重复或冲突的开放 PR。test-suit/axvisor/normal 下的板级测试布局与已有用例一致，没有重复。
阻塞问题
os/axvisor/src/vmm/fdt/parser.rs 中 try_get_host_fdt() 函数存在错误处理回归（见下方内联评论）。FdtHeader::from_bytes() 解析失败时使用了 .unwrap()，会导致 panic，而旧代码在此处优雅地返回 None 并记录错误日志。这与函数名 try_ 前缀的语义不符，且破坏了调用方（如 handle_fdt_operations）对 None 返回值进行优雅降级的预期。
请恢复为原版的错误处理模式：解析失败时记录 error! 日志并 return None，而不是 panic。
非阻塞观察
get_host_fdt()wrapper 的移除是正确的：该函数原本只是 panic wrapper，现在调用方直接使用try_get_host_fdt()并正确处理Option，设计更好。- 板级测试 smoke 配置中的
success_regex/fail_regex符合项目惯例。
Powered by deepseek-v4-pro
| }; | ||
| let fdt_header = FdtHeader::from_bytes(header) | ||
| .map_err(|e| format!("Failed to parse FDT header: {e:#?}")) | ||
| .unwrap(); |
There was a problem hiding this comment.
此处 .unwrap() 在 FDT header 解析失败时会 panic，破坏了 try_get_host_fdt() 函数返回 Option 的容错语义。
旧代码使用 match 在 Err 分支记录 error! 日志并 return None，允许调用方（如 handle_fdt_operations）优雅降级。请恢复为原版错误处理方式：
let fdt_header = match FdtHeader::from_bytes(header) {
Ok(header) => header,
Err(e) => {
error!("Failed to parse host FDT header: {e:#?}");
return None;
}
};修复后，调用方在 try_get_host_fdt() 返回 None 时可以继续正常执行（如 loongarch64 路径和 handle_fdt_operations 中的 warn! 分支），无需担心 panic。
d888f46 to
23575ca
Compare
There was a problem hiding this comment.
审查总结
PR #934 为 Axvisor 新增 PhytiumPi 和 ROC-RK3568-PC 板级测试，改进 FDT CPU 映射逻辑，并让 riscv64 runtime FDT 修补支持可选的 host FDT。上一轮的阻塞问题（try_get_host_fdt() 中 .unwrap() panic 风险）已在本提交中修复。审查通过。
变更概览
- 新增板级测试用例：
phytiumpi-linux和roc-rk3568-pc-linux，遵循现有test-suit/axvisor/normal布局。 - 板级驱动特征更新：PhytiumPi 使用
phytium-mci，ROC-RK3568-PC 使用rockchip-sdhci。 - CI 变更：
- 简化 loongarch64 QEMU CI 为统一命令
cargo xtask axvisor test qemu --arch loongarch64 - 新增两块自托管板级 CI（仅
rcore-os仓库触发，不限于 main PR）
- 简化 loongarch64 QEMU CI 为统一命令
- FDT CPU 映射重构：
- 分离 host CPU node 选择（基于
/cpus/cpu@Xunit address）与 guest-visible CPU ID（来自reg属性） - 新增
set_guest_phys_cpu_idssetter - 新增单元测试
cpu_node_selection_uses_node_id_when_reg_differs和cpu_node_id_parses_hex_unit_address
- 分离 host CPU node 选择（基于
- riscv64 runtime FDT 修补：
patch_guest_fdt_for_runtime的host_fdt参数改为Option<&Fdt>，缺失时只重建 memory node 而不再 panic - 上次阻塞问题修复：
try_get_host_fdt()中的FdtHeader::from_bytes()已恢复为match+return None的容错模式
验证结果
cargo fmt --check：通过cargo test -p axvm --lib：通过（2 个测试）- axvisor 新增单元测试（
cpu_node_id、cpu_node_selection_uses_node_id_when_reg_differs）：逻辑正确，测试覆盖了 node id 与 reg 不一致时的选择行为 - GitHub Actions CI（suite
70615554380）：结论 success
重复/冲突分析
- PR #859（draft，同作者 YanLien）：包含相同的 PhytiumPi/ROC-RK3568 板测内容外加 rsext4 恢复改进。#934 是从 #859 中拆分出来的聚焦版本，作为非 draft PR 提交。两者不冲突，属于有意拆分。
- base 分支：
test-suit/axvisor/normal下不存在board-phytiumpi或board-roc-rk3568-pc条目，均为全新内容。 - PR #930（x86_64 Linux guest）：与板测和 FDT CPU 映射无重叠。
- PR #936（UTS namespace）：完全无关。
无发现重复或冲突。
非阻塞观察
get_host_fdt()wrapper 的移除是正确的：该函数原本只是 panic wrapper，现在所有调用方直接使用try_get_host_fdt()并正确处理Option，设计更合理。- 板级测试 smoke 配置中的
success_regex/fail_regex符合项目惯例。 - CI 中新增的自托管板级 job 使用了
required_repository_owner: rcore-os限制，避免在 fork PR 上触发缺少板子的 job，这是正确的做法。
上一轮 review thread
上一轮由 mai-team-app[bot] 在 d888f469 提交上提出的 try_get_host_fdt() 中 .unwrap() 问题已在本提交（23575cae）中修复。当前代码使用 match FdtHeader::from_bytes(header) { Ok => ..., Err => { error!(...); return None; } } 的正确容错模式。由于该 inline comment 指向已过期的 commit/行号，无需额外操作。
Powered by deepseek-v4-pro
There was a problem hiding this comment.
审查总结
PR #934 为 Axvisor 新增 PhytiumPi 和 ROC-RK3568-PC 板级测试，改进 FDT CPU 映射逻辑，并让 riscv64 runtime FDT 修补支持可选的 host FDT。这是从 #859（draft）中拆分出的聚焦版本，审查通过。
变更概览
- 新增板级测试用例：
phytiumpi-linux和roc-rk3568-pc-linux，遵循现有test-suit/axvisor/normal布局，包含build-*.toml和smoke/*.toml配置。 - 板级驱动特征更新：PhytiumPi 使用
phytium-mci，ROC-RK3568-PC 使用rockchip-sdhci（同时启用rockchip-soc），替换了泛化的sdmmcfeature。 - CI 变更：
- 简化 loongarch64 QEMU CI 为统一命令
cargo xtask axvisor test qemu --arch loongarch64，删除了手动的多步配置脚本。 - 新增两块自托管板级 CI job（
roc-rk3568-pc-linux和phytiumpi-linux），使用required_repository_owner: rcore-os限制避免在 fork PR 上触发。
- 简化 loongarch64 QEMU CI 为统一命令
- FDT CPU 映射重构：
- 新增
cpu_node_id()从/cpus/cpu@Xunit address 提取 node ID。 - 新增
cpu_reg_address()从 CPU node 的reg属性提取地址。 need_cpu_node()优先使用 node_id 匹配phys_cpu_ids，fallback 到 reg address。set_phys_cpu_sets()现在同时设置phys_cpu_sets和guest_phys_cpu_ids，分离 host CPU node 选择与 guest-visible CPU ID。- 新增
set_guest_phys_cpu_ids()setter（components/axvm/src/config.rs）。 - 新增单元测试
cpu_node_selection_uses_node_id_when_reg_differs和cpu_node_id_parses_hex_unit_address。
- 新增
- riscv64 runtime FDT 修补：
update_fdt()使用try_get_host_fdt()（而非get_host_fdt()panic wrapper）。patch_guest_fdt_for_runtime()的host_fdt参数改为Option<&Fdt>，缺失时只重建 memory node，不再 panic。
- 错误处理修复（上次阻塞问题）：
try_get_host_fdt()中的FdtHeader::from_bytes()已恢复为match+return None的容错模式。get_host_fdt()panic wrapper 已移除。
验证结果
cargo fmt --check：通过，无格式问题。cargo test -p axvm --lib：通过（2 个已有测试正常）。- 新增 FDT CPU node 单元测试逻辑审查：
cpu_node_selection_uses_node_id_when_reg_differs正确验证了 node_id（0x100）与 reg（0x0）不同时，优先按 node_id 选择 CPU 节点的行为。 - GitHub Actions CI（suite
70615554380）：conclusionsuccess。
重复/冲突分析
- PR #859（draft，同作者 YanLien）：包含相同的板测内容以及 rsext4 恢复改进。#934 是从 #859 中有意拆分出的聚焦版本（仅 Axvisor 板测 + FDT CPU 映射），作为非 draft PR 提交。两者不冲突。
- PR #937（ZR233，driver/static probe 重构）：完全无关，无重叠。
- PR #936（fzg-23，UTS namespace）：完全无关，无重叠。
- base 分支：
test-suit/axvisor/normal下不存在board-phytiumpi或board-roc-rk3568-pc条目，均为全新内容。无重复。
上一轮 review thread
上一轮由 mai-team-app[bot] 在 commit d888f469 上提出的 try_get_host_fdt() 中 .unwrap() 问题已在当前 commit（23575cae）中修复。当前代码使用 match FdtHeader::from_bytes(header) { Ok => ..., Err => { error!(...); return None; } } 的正确容错模式。该 inline comment 指向已过期的旧 commit/行号，无需额外操作。
非阻塞观察
get_host_fdt()panic wrapper 的移除是正确的设计改进：所有调用方现在直接使用try_get_host_fdt()并正确处理Option。- 板级测试 smoke 配置中的
success_regex/fail_regex符合项目惯例，phytiumpi 使用phytiumpi login:匹配，roc-rk3568-pc 使用login:匹配。 - CI 中新增的自托管板级 job 使用了
required_repository_owner: rcore-os限制，避免在 fork PR 上触发缺少物理板子的 job，这是正确的做法。 - ROC-RK3568-PC 的 build config 额外启用了
rockchip-socfeature，与板子实际需求匹配。 maintainer_can_modify=false且mergeable_state=blocked：分支保护规则可能要求特定 reviewer 批准或等待其他 CI 通过，但不影响代码审查结论。
Powered by deepseek-v4-pro
ZR233
left a comment
ZR233
left a comment
There was a problem hiding this comment.
这两个新增 self-hosted board job 需要修改后再合入。
当前 phytiumpi-linux 和 roc-rk3568-pc-linux 写的是 required_repository_owner: rcore-os，但 .github/workflows/reusable-command.yml 只声明并检查 limit_to_owner，.github/workflows/ci.yml 调用 reusable workflow 时也只传 matrix.limit_to_owner。因此这个字段不会生效，不能保证这两个只有 rcore-os 组织才有硬件的 board 测试在其他 fork 中被跳过。
建议改法：
- name: Test axvisor self-hosted board roc-rk3568-pc-linux
use_container: false
runs_on: '["self-hosted","linux","board"]'
command: cargo xtask axvisor test board --board roc-rk3568-pc-linux
cache_key: ""
container_image: ""
limit_to_owner: rcore-os
main_pr_only: false
- name: Test axvisor self-hosted board phytiumpi-linux
use_container: false
runs_on: '["self-hosted","linux","board"]'
command: cargo xtask axvisor test board --board phytiumpi-linux
cache_key: ""
container_image: ""
limit_to_owner: rcore-os
main_pr_only: false如果目标还包括“外部 fork 提到 rcore-os 的 PR 也不运行这两个 board 测试”，仅替换成 limit_to_owner 还不够，因为 PR 的 github.repository_owner 仍然是 rcore-os。这种场景需要再加一个显式的 head repo owner 条件，例如给 reusable workflow 增加输入并在 run_host / run_container 的 if 中要求 github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == inputs.<trusted-owner>；或者在 matrix/with 层为这两个硬件 job 做等价的跳过逻辑。注意现有 self_hosted_owner 逻辑是把 self-hosted job 改到 ubuntu/container，不是跳过 board command，不能直接用于这两个必须依赖实体板子的测试。
23575ca to
4a50a41
Compare
2 participants
Problem
Axvisor board-test coverage did not include PhytiumPi or ROC-RK3568-PC, so their Linux guest boot paths were not covered by the unified
cargo xtask axvisor test boardflow or self-hosted board CI.These boards also need more precise FDT CPU handling. The previous logic could mix host CPU node unit addresses with guest-visible CPU IDs, which breaks CPU node filtering and vCPU mapping on boards where those values differ.
Changes
phytiumpi-linuxroc-rk3568-pc-linuxphytium-mcirockchip-sdhcicargo xtask axvisor test qemu --arch loongarch64/cpus/cpu@...unit address when available.reg.axvmsetter for guest physical CPU IDs.try_get_host_fdt():/chosenfrom the host FDT when available.Implementation Notes
The new board tests follow the existing
test-suit/axvisor/normaldiscovery layout, so they can be run through the same board-test path as the existing Axvisor board cases.CPU handling now separates host CPU node selection from guest CPU ID exposure. The host FDT unit address decides which CPU nodes are kept, while the node
regvalue is written back as the guest-visible CPU ID. This keeps vCPU placement and guest FDT generation consistent on boards where those values differ.For the riscv64 runtime FDT patch path, host FDT access is now optional. When a host FDT is available, missing
/chosenmetadata can still be restored from it; otherwise the patcher keeps the existing cached guest FDT content and still rebuilds the runtime memory node.