refactor(axvm): consolidate architecture-specific code - #1562
Conversation
There was a problem hiding this comment.
评审总结
本 PR 将 AxVM 的架构差异代码从 boot、host、irq、runtime、timer、VM 准备流程和 crate 根模块中收束到 virtualization/axvm/src/arch/ 目录,是一次高质量的架构重构。
代码质量评估
设计方面:
- 新增的
architecture合约层设计合理,GuestBootPlatform、BootImagePlatform、DevicePlatform、AddressSpacePlatform、HostTimePlatform五个能力接口职责清晰、边界明确。 PreparedGuestBoottyped state 模式有效地将配置 enrichment、VM 创建和镜像装载之间的所有权关系类型化,避免了 DTB 与配置被拆散传递的风险。- FDT 拆分为
boot/fdt/core(共享机制)+ 各架构fdt.rs(策略函数)的层次清晰,AArch64 的 GIC interrupt specifier、RISC-V 的 PLIC 解码、LoongArch 的 UEFI 门面、x86_64 的不支持标记各自归位。 HostTimetrait 精简为只保留 monotonic clock 与 one-shot timer,timer callback 注册细节留在架构实现,符合精简原则。
边界契约:
- 10 个集成/边界契约测试设计出色,覆盖了
target_arch位置约束、架构目录互斥性、公共模块反向依赖检查、Axvisor 统一门面使用、HostTime接口精简等关键不变量。 - 这些测试是回归防护的核心,确保后续改动不会破坏架构边界。
Axvisor 消费者端:
os/axvisor/src/config.rs中移除了所有#[cfg(target_arch = "...")]的 AxVM 启动分支,统一使用prepare_guest_boot+prepared_boot.load_images()流程,代码简化且逻辑集中。
本地验证结果
cargo fmt --check:通过cargo xtask clippy --package axvm:6/6 通过cargo test -p axvm:92 个单元测试 + 10 个集成/边界测试全部通过cargo xtask clippy --package axvisor:跳过(需 Axvisor target 配置)- 未发现
[patch.crates-io]覆盖
CI 状态
PR 当前 head (553aa8b) 尚无 CI check run 记录(状态为 pending,无 check runs),PR 提交时间较短。建议等待 CI 完成后确认所有架构的 axvisor build 通过。
重复/重叠 PR 分析
- 搜索了
axvm+arch+refactor相关 open PR,未发现重复或冲突的 PR。 - 与 PR #1451(IRQ runtime 重构)无重叠,属于不同领域。
阻塞问题:合并冲突
当前 PR 与 origin/dev 存在合并冲突(mergeable_state: dirty),涉及两个文件:
-
virtualization/axvm/src/arch/mod.rs:PR 将BoundVcpuExit、VcpuRunAction等类型移入architecture/模块,而 dev 分支上 PR #1528(vCPU exit 处理重构)在此文件新增了LegacyDeferredRunWork等类型。冲突发生在架构分发函数与遗留类型定义的位置。 -
virtualization/axvm/src/arch/x86_64/mod.rs:dev 分支的 PR #1556(x86 vCPU OS-neutral)为VcpuSetupContext新增了memory_regions字段,而 PR 使用into_parts()解构方式访问该上下文,缺少对新字段的处理。
由于 dev 分支近期合并了多个架构相关 PR(#1528、#1550、#1555、#1556),冲突涉及语义层面的适配(LegacyDeferredRunWork 归位、memory_regions 字段集成),不建议 reviewer 直接代为修复。请 rebase 到最新 origin/dev 并解决冲突后重新提交。
建议
- Rebase 到最新
origin/dev,解决arch/mod.rs和arch/x86_64/mod.rs的冲突。 - 将 dev 新增的
LegacyDeferredRunWork类型归入architecture/模块。 - 在
VcpuSetupContext中补充memory_regions字段(来自 PR #1556)。 - Rebase 后重新运行
cargo xtask clippy --package axvm和cargo test -p axvm确认通过。 - 等待 CI 在所有架构上通过。
评审结论
代码重构质量优秀,架构边界清晰,测试覆盖充分。当前唯一阻塞项是合并冲突。请在 rebase 后重新请求评审。
Powered by deepseek-v4-pro
553aa8b to
549bbcb
Compare
There was a problem hiding this comment.
评审总结
本 PR 将 AxVM 的架构差异代码从 boot、host、irq、runtime、timer、VM 准备流程和 crate 根模块中收束到 virtualization/axvm/src/arch/ 目录,是一次高质量、行为保持的架构重构。
代码质量评估
设计方面:
- 新增的
architecture合约层设计合理,GuestBootPlatform、BootImagePlatform、DevicePlatform、AddressSpacePlatform、HostTimePlatform五个能力 trait 职责清晰、边界明确,符合 Rust 组合优于继承的设计原则。 PreparedGuestBoottyped state 模式有效地将配置 enrichment、VM 创建和镜像装载之间的所有权关系类型化,避免了 DTB 与配置被拆散传递的风险。- FDT 拆分为
boot/fdt/core(共享机制)+ 各架构fdt.rs(策略函数)的层次清晰:AArch64 的 GIC interrupt specifier、RISC-V 的 PLIC 解码、LoongArch 的 UEFI 门面、x86_64 的不支持标记各自归位在正确的架构目录中。 HostTimetrait 精简为只保留monotonic_time()与set_oneshot_timer()两个方法,timer callback 注册细节留在HostTimePlatform架构 hook 中,符合精简原则。arch/mod.rs作为分发页只包含target_arch条件编译选择 + 薄转发函数,通用代码不再直接维护#![cfg(target_arch = ...)]分支。
Axvisor 消费者端:
os/axvisor/src/config.rs中移除了所有#[cfg(target_arch = "...")]的 AxVM 启动分支,统一使用prepare_guest_boot+prepared_boot.load_images()流程,代码大幅简化且逻辑集中。- 原有的
VMBootProtocol、is_x86_linux_image_config、handle_fdt_operations、init_guest_boot_resources等函数调用全部通过统一定义的boot模块门面完成,不再需要消费者做架构分支。
边界契约:
- 12 个集成/边界契约测试设计出色,覆盖了
target_arch位置约束、架构目录互斥性、公共模块反向依赖检查、Axvisor 统一门面使用、HostTime接口精简等关键不变量。 - 这些测试是回归防护的核心,确保后续改动不会破坏架构边界。
代码规范:
vm.rs→vm/mod.rs + vm/*目录结构迁移符合项目规范(book/guideline/code-quality.md§7.7)。- 模块默认私有(
pub(crate)),按需公开,符合可见性最小化原则。 - 源文件阅读顺序遵循"入口在前、步骤向下展开、测试在最后"的报纸式结构。
本地验证结果
cargo fmt --check:通过cargo xtask clippy --package axvm:全部 6 个 feature 组合通过(default / fs / host-fs / sstc / svm / vmx),-D warningscargo test -p axvm:93 个单元测试 + 12 个集成/边界契约测试全部通过- 未发现工作区内任何
[patch.crates-io]覆盖 - 与
origin/dev合并测试无冲突(Already up to date)
CI 状态
PR 当前 head (549bbcb) 的 CI run #29084588167 正在执行中(status: in_progress),尚在早期阶段。CI 使用路径过滤,本 PR 仅修改 virtualization/axvm/ 和 os/axvisor/src/config.rs,预期不会触发 Starry/ArceOS QEMU 测试。建议关注 axvisor build 相关 job 的结果。
重复/重叠 PR 分析
- 搜索了
axvm+arch+refactor相关 open PR,未发现重复或冲突的 PR。 - PR #1451(starry irq runtime 重构)与本 PR 无重叠,属于不同领域。
上一轮评审回顾
前次评审(commit 553aa8b)提出的以下问题在本轮已解决:
- 合并冲突:PR 已 rebase 到最新
origin/dev,与 dev 合并无冲突。 LegacyDeferredRunWork归位:x86_64 的DeferredRunWork已归入arch/x86_64/exit.rs。memory_regions字段集成:x86_64 的build_vcpu_setup_config()现在接收memory_regions: &[VMMemoryRegion]参数并正确传递给X86VCpuSetupConfig。VcpuSetupContext移除:已按 PR 设计目标移除五元 union,架构适配器改为直接接收具名的 VM config 和 memory_regions。
评审结论
代码重构质量优秀,架构边界清晰,测试覆盖充分。所有本地验证通过,无阻塞问题。批准合并。
建议合并后关注 CI 中 axvisor 各架构的 build job 是否全部通过。
Powered by deepseek-v4-pro
549bbcb to
d074909
Compare
There was a problem hiding this comment.
评审总结
本 PR 将 AxVM 的架构差异代码从 boot、host、irq、runtime、timer、VM 准备流程和 crate 根模块中收束到 virtualization/axvm/src/arch/ 目录,是一次高质量、行为保持的架构重构。
代码质量评估
设计方面:
- 新增的
architecture合约层设计合理,GuestBootPlatform、BootImagePlatform、DevicePlatform、AddressSpacePlatform、HostTimePlatform五个能力 trait 职责清晰、边界明确,符合项目 "组合优于继承" 的设计原则。 PreparedGuestBoottyped state 模式有效地将配置 enrichment、可选 DTB 与镜像装载之间的所有权关系类型化,避免了 DTB 与配置被拆散传递的风险。- FDT 拆分为
boot/fdt/core(共享机制)+ 各架构fdt.rs(策略门面)的层次清晰:AArch64 的 GIC interrupt specifier、RISC-V 的 PLIC 解码、LoongArch 的 UEFI 门面、x86_64 的不支持标记各自归位在正确的架构目录中。 HostTimetrait 精简为只保留monotonic_time()与set_oneshot_timer()两个方法,timer callback 注册细节留在HostTimePlatform架构 hook 和timer.rs中,符合精简原则。arch/mod.rs作为分发页只包含target_arch条件编译选择 + 薄转发函数,通用代码不再直接维护#[cfg(target_arch = ...)]分支。
Axvisor 消费者端:
os/axvisor/src/config.rs中移除了所有#[cfg(target_arch = "...")]的 AxVM 启动分支,统一使用prepare_guest_boot+prepared_boot.load_images()流程,代码大幅简化且逻辑集中。- 原有的
handle_fdt_operations、is_x86_linux_image_config、init_guest_boot_resources、DEFAULT_X86_BIOS_LOAD_GPA等分散调用全部通过统一的boot模块门面完成,不再需要消费者做架构分支。
边界契约测试:
- 12 个集成/边界契约测试设计出色,覆盖了
target_arch位置约束、架构目录互斥性、公共模块反向依赖检查、Axvisor 统一门面使用、HostTime接口精简、VcpuSetupContext移除等关键不变量。 - 这些测试是回归防护的核心,确保后续改动不会破坏架构边界。
代码规范:
vm.rs→vm/mod.rs + vm/*目录结构迁移符合项目规范(book/guideline/code-quality.md§0.1 报纸式结构)。- 模块默认私有(
pub(crate)),按需公开,符合可见性最小化原则。
本地验证结果
cargo fmt --check:通过cargo xtask clippy --package axvm:全部 6 个 feature 组合通过(default / fs / host-fs / sstc / svm / vmx),-D warningscargo test -p axvm:93 个单元测试 + 12 个集成/边界契约测试全部通过- 未发现工作区内任何
[patch.crates-io]覆盖
CI 状态
PR 当前 head (d074909) 的 CI run #29218498167:
Check formatting:successRun spin-lint:successRun sync-lint:successRun clippy:successTest starry aarch64 qemu:success- 多个 axvisor self-hosted board 测试(phytiumpi-linux、orangepi-5-plus-linux、asus-nuc15crh-linux、roc-rk3568-pc-linux):全部 success
Test arceosjobs:success- 其他 job(Starry x86_64/loongarch64、axvisor riscv64 等):skipped(路径过滤,本 PR 未触及这些区域,属正常行为)
无 CI 失败项,无 PR 导致的 CI 问题。
重复/重叠 PR 分析
- 搜索了
axvm+arch+refactor相关 open PR,未发现重复或冲突的 PR。 - PR #1322(axdevice registry)属于设备框架领域,无重叠。
- PR #1451(starry irq runtime 重构)属于调度/IRQ 领域,无重叠。
上一轮评审回顾
前次评审(commit 553aa8b,CHANGES_REQUESTED)提出的合并冲突问题已在本轮解决:
- PR 已 rebase 到最新
origin/dev,当前mergeable_state: clean - x86_64 的
build_vcpu_setup_config()正确接收memory_regions: &[VMMemoryRegion]参数(保留了上游 #1555 的 guest memory region 传递) VcpuSetupContext已移除,架构适配器改为直接接收具名的 VM config 和 memory_regions
评审结论
代码重构质量优秀,架构边界清晰,测试覆盖充分。所有本地验证和 CI 均通过,无阻塞问题。批准合并。
Powered by deepseek-v4-pro
There was a problem hiding this comment.
评审总结
本 PR 将 AxVM 的架构差异代码收束到 virtualization/axvm/src/arch/ 目录,是一次高质量的架构重构。当前 head (543d03c) 在上一次已批准版本 (d074909) 基础上进一步将 VM 初始化编排集中到各架构的 vm.rs 中,边界契约测试从 12 个增长到 18 个,清理更加彻底。
代码质量评估
设计方面:
architecture合约层设计合理,GuestBootPlatform、BootImagePlatform、HostTimePlatform三个能力 trait 职责清晰、边界明确,符合项目"组合优于继承"的设计原则。arch/mod.rs作为分发页仅包含target_arch条件编译选择 + 薄转发函数,通用代码不再直接维护#[cfg(target_arch = ...)]分支。- 四架构 VM 初始化 (x86_64/AArch64/RISC-V/LoongArch) 各自在
arch/<target>/vm.rs中独立编排 vCPU 创建、设备注册、地址空间映射和资源提交流程,通用层只通过CurrentArch::create_vm_resources与CurrentArch::init_vm两个入口调度。 - 模块默认私有 (
pub(crate)),按需公开,符合可见性最小化原则。
Axvisor 消费者端:
os/axvisor/src/config.rs中移除了所有#[cfg(target_arch = "...")]的 AxVM 启动分支,统一使用prepare_guest_boot+prepared_boot.load_images()流程,代码大幅简化。
边界契约测试:
- 18 个集成/边界契约测试覆盖了
target_arch位置约束、架构目录互斥性、公共模块反向依赖检查、Axvisor 统一门面使用、HostTime接口精简、VcpuSetupContext移除、四架构 VM 初始化所有权、eager Ready 生命周期、失败清理等关键不变量。 - 这些测试是回归防护的核心。
上一轮评审回顾
- 第一轮评审 (commit
553aa8b,CHANGES_REQUESTED) 提出的合并冲突问题已在第二轮解决。 - 第二轮和第三轮评审 (commits
d074909/549bbcb) 均已 APPROVE,无遗留阻塞问题。 - 本轮新增提交
543d03c进一步集中 VM 初始化编排 + 增加 6 个边界契约测试,属于增量改进,未引入回归。
本地验证结果
cargo fmt --check:通过cargo xtask clippy --package axvm:全部 6 个 feature 组合通过(default / fs / host-fs / sstc / svm / vmx),-D warningscargo test -p axvm:92 个单元测试 + 18 个集成/边界契约测试全部通过- 未发现工作区内任何
[patch.crates-io]覆盖
CI 状态
PR 当前 head (543d03c) 的 CI run #29222308028 全部通过(28+ jobs 成功,0 失败):
Check formatting:successRun spin-lint:successRun sync-lint:successRun clippy:successTest starry(x86_64, loongarch64, aarch64, riscv64) qemu:全部 successTest axvisor(aarch64, loongarch64, riscv64, x86_64 svm hosted) qemu/board:全部 successTest arceos(x86_64, aarch64, riscv64, loongarch64) qemu:全部 success- Self-hosted board 测试(orangepi-5-plus, phytiumpi, roc-rk3568-pc, asus-nuc15crh, aka-00-sg2002, visionfive2):全部 success
Test with std:successTest axloader HTTP smoke:success
无 CI 失败项,无 PR 导致的 CI 问题。部分 job(publish container image, 互斥的 run_container/run_host)按预期 skipped。
重复/重叠 PR 分析
- 搜索了
axvm+arch+refactor相关 open PR,未发现重复或冲突的 PR。 - 本 PR 是独特的架构重构,无其他 PR 覆盖相同范围。
评审结论
代码重构质量优秀,架构边界清晰,测试覆盖充分。所有本地验证和 CI 均通过,无阻塞问题。批准合并。
Powered by deepseek-v4-pro
问题
AxVM 的架构差异此前分散在 boot、host、IRQ、runtime、timer 与 VM 准备流程中,通用代码需要维护多处架构分支和细粒度 hook。VM 初始化尤其依赖
DevicePlatform、AddressSpacePlatform、VcpuCreateContext以及ArchOps中的 NPT/vCPU 构造接口,导致通用层需要了解不同架构的初始化步骤和顺序。同时,VM 资源此前由
Machine::Uninit延迟创建,配置访问可能隐式触发状态转换,初始化失败后的资源提交与 stage-2 临时映射清理也缺少统一边界。修改
target_arch分支收束到virtualization/axvm/src/arch/。src/arch根目录收敛为四个架构目录和目标分发页;通用 NPT、镜像字节装载和 FDT core 使用公共领域路径。architecture合约层,按职责保留 guest boot、boot image、host timer 与运行期能力边界。arch/<target>/vm.rs:CurrentArch::create_vm_resources与CurrentArch::init_vm两个生命周期级入口;默认和调用方提供 factories/fabric 的路径均经过同一架构初始化流程。AxVM::neweager 创建 NPT、stage-2 地址空间和嵌套分页配置,并直接返回Ready;删除 lazyensure_resources_ready、Machine::Uninit、VmStatus::Uninit和prepare_with。ArchOps移除 NPT 创建、vCPU create/setup 方法,并删除 VM-init 专用的DevicePlatform、AddressSpacePlatform与VcpuCreateContext。VmStatus::Uninit。实现逻辑
AxVM::new只负责取得当前架构创建的持久 VM 资源。prepare/prepare_with_factories把初始化请求整体交给CurrentArch,由各架构文件显式编排 vCPU、设备、地址布局和附加映射顺序。通用 prepare helper 仅维护生命周期校验、通用设备/layout 机械操作以及原子式资源提交,不表达架构策略。boot/FDT/image、host/per-CPU/timer 和运行期 exit 仍保持独立能力边界,未并入 VM init;已有镜像加载、VM 管理和架构兼容导出路径保持不变。
VmStatus::Uninit的删除为本次明确接受的公共 API 变更。冲突处理
分支已 rebase 到最新
origin/dev。上游 #1555 新增的 x86 guest memory region 传递被保留并迁移到架构 VM setup;本次没有手工合并Cargo.lock。验证
cargo fmt --all --checkcargo xtask clippy --package axvm(6/6 通过,-D warnings)cargo test -p axvm --tests(92 个单元测试、18 个集成/边界测试通过)cargo xtask axvisor build --arch x86_64cargo xtask axvisor build --arch aarch64cargo xtask axvisor build --arch riscv64cargo xtask axvisor build --arch loongarch64cargo xtask axvisor test qemu --arch x86_64 --test-group normal --test-case smoke-vmx(1/1 通过)git diff --check