Skip to content

feat(axdevice): introduce unified device model with registry and factory-based GPPTRedistributor migration#1322

Closed
baitwo02 wants to merge 6 commits into
rcore-os:devfrom
baitwo02:dev
Closed

feat(axdevice): introduce unified device model with registry and factory-based GPPTRedistributor migration#1322
baitwo02 wants to merge 6 commits into
rcore-os:devfrom
baitwo02:dev

Conversation

@baitwo02

Copy link
Copy Markdown

解决的问题

当前 AxVisor 设备框架存在两个结构性问题:

  1. 设备分发路径分散:MMIO/PIO/SysReg 访问各自走独立的 Vec 线性查找,没有统一的路由和冲突检查。
  2. 设备创建逻辑耦合在核心容器AxVmDevices::init() 里一个巨大的 match 语句直接构造各个具体设备类型。新增设备必须修改 axdevice 核心代码,设备 crate 和核心容器耦合在一起。

改动内容

Commit 1:统一设备注册表 (feat(axdevice): add unified device registry)

  • 新增 axdevice_base crate,承载设备模型基础组件:
    • DeviceOps trait — 设备统一接口,声明 id/name/resources/capabilities/access
    • BusAccess / BusResponse / BusAddress / BusKind — 总线访问抽象
    • Resource::Mmio/Pio/SysReg/Irq/Msi/Dma/PciBar — 设备资源声明
    • DeviceId / DeviceError / DeviceCapabilities / DeviceMeta / IrqSink
  • 新增 DeviceRegistry,提供:
    • register_device(Rc<dyn DeviceOps>) — 注册设备并建立 MMIO/PIO/SysReg 路由表
    • dispatch(BusAccess) — 统一的总线访问分发(不再走旧 Vec 线性查)
    • 注册时检查重复 DeviceId
    • 注册时检查同一 bus kind 下的资源重叠
  • 新增 LegacyDeviceAdapter,将旧 BaseMmioDeviceOps / BasePortDeviceOps / BaseSysRegDeviceOps 适配为 DeviceOps,使未迁移设备也能进入新注册表。
  • AxVmDevices::add_mmio_dev/add_port_dev/add_sys_reg_dev 内部改为同时注册到 DeviceRegistry(旧 Vec 保留用于 typed control path)。
  • AxVmDevices::handle_mmio_read/write 等六个入口收敛到 dispatch_bus_access()
  • 新增 axdevice 单元测试 1142 行,覆盖 registry 注册、路由、冲突检查、dispatch、read/write 响应类型不匹配等。

Commit 2:原生 VGicR factory 迁移 (feat(axdevice): add native VGicR factory path)

  • 定义 DeviceFactory trait(ty() + build(ctx, config) -> Vec<Rc<dyn DeviceOps>>
  • 定义 DeviceBuildContext trait(alloc_device_id()
  • 定义 DeviceFactoryCatalog,按 EmulatedDeviceType 查找 factory
  • 新增 GpptRedistributorFactory
    • 解析 cfg_list = [cpu_num, stride, pcpu_id]
    • 一个 config 展开为 N 个 VGicR 实例
    • 参数缺失时返回结构化错误,不使用 expect()/panic!()
  • VGicR 重构:
    • 直接实现 DeviceOps(不再通过 LegacyDeviceAdapter 包装)
    • 删除 impl BaseDeviceOps<GuestPhysAddrRange>
    • 删除 legacy_meta / new_with_meta
    • 持有 DeviceMeta 并通过 DeviceOps 方法转发
  • 新增 init_from_aarch64_catalog() — AArch64 设备初始化入口,先尝试 catalog 查找 factory,找不到则 fallback 到原有 match 路径
  • AxVmDevices::init()GPPTRedistributor 的 match 分支改为输出警告("not supported by active factory catalog"),实际创建由 factory 完成
  • 新增 AArch64 device migration 测试组 test-suit/axvisor/aarch64-device-migration/qemu/smoke,通过 QEMU smoke 测试验证 factory → build → registry dispatch 完整链路

设计原则

  • factory 返回 Vec<Rc<dyn DeviceOps>>,不暴露 BaseDeviceOps
  • 迁移设备默认直接实现 DeviceOps,不新增 wrapper
  • catalog 描述"有哪些可创建设备",registry 描述"已注册哪些设备",职责分离
  • 旧 Vec 和 LegacyDeviceAdapter 作为迁移期兼容层保留,新设备不走此路径

验证情况

  • cargo xtask clippy --package axdevice 通过
  • cargo xtask clippy --package axdevice_base 通过
  • cargo xtask axvisor test qemu --arch aarch64 --test-group aarch64-device-migration --test-case smoke 通过
  • axdevice 单元测试覆盖 registry 注册/路由/冲突/dispatch 等核心路径
  • 非 AArch64 构建不受影响(cfg 隔离)

Copilot AI review requested due to automatic review settings June 20, 2026 08:57
@baitwo02 baitwo02 changed the title feat(axdevice): 引入统一设备模型,基于 registry 和 factory 完成 GPPTRedistributor 迁移 feat(axdevice): introduce unified device model with registry and factory-based GPPTRedistributor migration Jun 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a unified emulated-device model for AxVisor by adding a registry-based routing layer (MMIO/PIO/SysReg) plus a factory/catalog mechanism for device construction, and migrates AArch64 GPPT GICv3 redistributors (VGicR) onto the native DeviceOps path.

Changes:

  • Add axdevice_base device model primitives (DeviceOps, BusAccess/BusResponse, Resource, DeviceError, IRQ abstractions) and implement DeviceRegistry with conflict checks + unified dispatch.
  • Introduce DeviceFactory + DeviceFactoryCatalog, migrate VGicR to native DeviceOps, and add GpptRedistributorFactory for config-driven multi-instance creation.
  • Update AxVM/axdevice dispatch paths and add extensive unit tests + a new Axvisor QEMU smoke test group for the migration chain.

Review Checklist (review-single-pr)

  • PR intent/description reviewed against code changes (diff + provided context)
  • All changed files in this submission were inspected for correctness and integration impact
  • CI/checks and existing review threads verified (not available from diff-only input; cannot confirm)
  • Local validation commands rerun (not possible in this environment; only author-provided results available)

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
virtualization/axvm/src/vm.rs Switch VM-exit MMIO/PIO/SysReg handling to the unified dispatch_bus_access path.
virtualization/axdevice/tests/test.rs Expand unit coverage for registry routing, conflicts, adapters, factories, and non-panicking errors.
virtualization/axdevice/src/registry.rs New DeviceRegistry with route tables, dispatch, and overlap/duplicate-ID checks.
virtualization/axdevice/src/lib.rs Re-export new device model types, registry, factory catalog, and legacy adapter.
virtualization/axdevice/src/legacy.rs Add LegacyDeviceAdapter bridging old Base*DeviceOps traits into DeviceOps.
virtualization/axdevice/src/factory.rs Add DeviceFactoryCatalog lookup by emulated device type.
virtualization/axdevice/src/device.rs Integrate registry into AxVmDevices, add native registration + unified dispatch entrypoints.
virtualization/axdevice_base/src/resource.rs Define Resource, DeviceCapabilities, and PCI BAR kind declarations.
virtualization/axdevice_base/src/model.rs Define core DeviceId, DeviceMeta, DeviceError, DeviceOps, and DeviceFactory traits.
virtualization/axdevice_base/src/lib.rs Wire up and re-export the new base modules (bus/irq/model/resource).
virtualization/axdevice_base/src/irq.rs Add architecture-neutral IRQ signaling types (IrqLine, IrqSink, MsiMessage).
virtualization/axdevice_base/src/bus.rs Add unified bus transaction types (BusAccess, BusAddress, BusKind, BusOp, BusResponse).
virtualization/arm_vgic/src/v3/vgicr.rs Migrate VGicR to native DeviceOps and add GpptRedistributorFactory.
test-suit/axvisor/aarch64-device-migration/qemu/smoke/qemu-aarch64.toml New QEMU smoke case validating factory→registry→dispatch chain.
test-suit/axvisor/aarch64-device-migration/qemu/build-aarch64-unknown-none-softfloat.toml Build config for the new Axvisor test group.
os/axvisor/configs/vms/qemu/aarch64/linux-smp1-gppt-gic.toml VM config enabling GPPT GICR emulation for migration testing.

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

Comment thread virtualization/axdevice/src/device.rs Outdated
Comment on lines +499 to +501
pub fn register_device(&mut self, device: Rc<dyn DeviceOps>) -> DeviceResult<DeviceId> {
self.registry.register_device(device)
}

@mai-team-app mai-team-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查总结

PR 内容:引入统一设备模型,新增 axdevice_base 基础抽象(DeviceOpsBusAccess/BusResponseResourceDeviceFactory)、DeviceRegistry 路由表、LegacyDeviceAdapter 适配器,并将 VGicR 迁移为直接实现 DeviceOps 的原生设备。设计思路清晰,职责分离合理(catalog 负责"可创建",registry 负责"已注册")。

验证结果

  • cargo fmt --check
  • cargo xtask clippy --package axdevice
  • cargo xtask clippy --package axdevice_base
  • cargo test -p axdevice(16 个集成测试 + 4 个单元测试)✅
  • cargo test -p axdevice_base
  • 组织侧 CI 未触发 check runs(来自 fork 的 PR),本地验证全部通过。

既有评论:无此前 review 评论。

影响范围:改动覆盖 axdevice_base(新增)、axdevice(核心重构)、arm_vgic(VGicR 迁移)、axvm(VM exit 分发路径迁移)。旧 Vec 和 LegacyDeviceAdapter 作为兼容层保留,非 aarch64 构建不受影响(cfg 隔离)。架构上是渐进式迁移,风险可控。

相关 PR#1320 是一个更激进的 axbus 抽象层方案(已关闭),本 PR 的方案更务实。


需要修改的问题

1. device_error_to_ax_error / dispatch_device_read / dispatch_device_write 重复定义

axdevice/src/device.rsaxvm/src/vm.rs 中各有一份完全相同的 4 个辅助函数(device_error_to_ax_errorunexpected_device_responsedispatch_device_readdispatch_device_write)。如果后续 DeviceError 新增变体或分发逻辑变化,需要同步修改两处,容易遗漏。

建议:将这组函数(或至少 device_error_to_ax_error)作为 axdevicepub 辅助函数导出,axvm 直接复用。dispatch_device_read/write 因依赖 &AxVmDevices,可以保留在 axdevice 内部但改为 pub(crate) 并在 axvm 中通过 axdevice 的导出路径调用,或提供 AxVmDevices 上的公开辅助方法。

2. 与 dev 分支存在合并冲突

mergeable_state: dirty,需要 rebase 到最新 dev。

Powered by mimo-v2.5-pro

Comment thread virtualization/axvm/src/vm.rs Outdated
}
}

fn device_error_to_ax_error(error: DeviceError) -> AxError {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这组函数(device_error_to_ax_errorunexpected_device_responsedispatch_device_readdispatch_device_write)在 axdevice/src/device.rs 中有完全相同的副本。建议将 device_error_to_ax_errorunexpected_device_response 作为 axdevicepub 辅助函数导出,然后在 axvm 中直接 use axdevice::device_error_to_ax_error 复用,避免后续维护时两处不同步。

Comment thread virtualization/axdevice/src/device.rs Outdated
) {
let rw = if read { "read" } else { "write" };
trace!("emu_device {rw}: {addr_type} {addr:#x} in range {addr_range:#x} with width {width:?}")
fn device_error_to_ax_error(error: DeviceError) -> AxError {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的 4 个辅助函数与 axvm/src/vm.rs 中完全重复。建议将 device_error_to_ax_errorunexpected_device_response 改为 pub,供 axvm 直接导入,消除重复。

@ZR233 ZR233 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当前 head:8df2316a3ed7e00574fe61c52f95d8ba6a73c6ee。

该 PR 目前还不能合入:

  1. mergeStateStatus 仍为 DIRTY,与最新 dev 存在合并冲突,需要先 rebase/merge dev 并解决冲突后再评审最终 diff。
  2. 仍有未解决的当前 review thread。其中 virtualization/axdevice/src/device.rs:499 指出的 register_device() 不同步推进 next_device_id 是实际行为风险:如果外部注册了较小的 native DeviceId,之后 add_mmio_dev/add_port_dev/add_sys_reg_dev 走 legacy allocator 时可能再次分配同一个 id,并在 registry 注册阶段触发 DuplicateDeviceId panic。需要让手动注册成功后同步推进 allocator,或明确禁止/隔离手动 id 与 legacy 分配空间。

补充检查:我确认了当前 head 的相关代码和未解决线程;git diff --check origin/dev...HEAD 通过。由于当前分支存在合并冲突且已有阻塞问题,本轮没有继续跑完整 clippy/QEMU 验证。

Add a static device factory registration system using linker sections:

- Introduce DeviceFactoryRegister struct and register_device_factory! macro
  in axdevice_base, placing entries in .axdevice.factory linker section
- Extend DeviceFactoryCatalog with from_linker()/from_registers() constructors
  and find_unique() for duplicate detection
- Update runtime.ld with __saxdevice_factory/__eaxdevice_factory symbols
- Refactor aarch64 device initialization to use linker-collected factories
- Register GpptRedistributorFactory via the new macro in arm_vgic
- Add tests for factory registration lookup and duplicate rejection

@mai-team-app mai-team-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查总结

PR 内容:引入统一设备模型,新增 axdevice_base 基础抽象(DeviceOpsBusAccess/BusResponseResourceDeviceFactory)、DeviceRegistry 路由表、LegacyDeviceAdapter 适配器,并将 VGicR 迁移为直接实现 DeviceOps 的原生设备。设计思路清晰,职责分离合理(catalog 负责"可创建",registry 负责"已注册")。

本地验证结果

  • cargo fmt --check
  • cargo xtask clippy --package axdevice
  • cargo xtask clippy --package axdevice_base
  • cargo xtask clippy --package axvm(8 个 feature 组合)✅
  • cargo test -p axdevice(18 个测试)✅
  • cargo test -p axdevice_base(1 个测试 + 1 个 doctest)✅
  • 组织侧 CI 未触发 check runs(来自 fork 的 PR),本地验证全部通过。

既往评论分析

  1. Copilot:指出 register_device() 不推进 next_device_id —— 当前代码路径通过 factory 调用 ctx.alloc_device_id() 可以正确分配,但 public API 确实允许外部传入低 ID 导致后续冲突。建议在文档中明确约束或同步推进 allocator。
  2. mai-team-app(bot):指出 device_error_to_ax_error 等 4 个辅助函数在 axdeviceaxvm 中完全重复 —— 当前 HEAD 仍然存在此问题,代码完全相同。
  3. ZR233:指出 mergeable_state: DIRTYregister_device 未同步 —— 当前 HEAD 仍然存在这两个问题

与 dev 的合并冲突

当前 HEAD 60b18bb96fa3origin/dev 存在 7 个文件冲突

  • virtualization/axdevice/src/device.rs
  • virtualization/axdevice/src/factory.rs(add/add 冲突)
  • virtualization/axdevice/src/lib.rs
  • virtualization/axdevice/tests/test.rs(modify/delete:dev 删除,PR 修改)
  • virtualization/axdevice_base/src/irq.rs(add/add 冲突)
  • virtualization/axdevice_base/src/lib.rs
  • virtualization/axvm/src/vm.rs

dev 分支已合入 #1258 "Issue 595 device foundation",引入了 registration.rsfactory.rsirq.rs 等同类基础模块,与本 PR 高度重叠。需要 rebase 后解决冲突才能合入。

相关 PR#1320(已关闭的更激进 axbus 抽象层方案),本方案更务实。


需要修改的问题

  1. 合并冲突(阻塞):上述 7 个文件与 dev 存在冲突,其中 factory.rsirq.rs 为 add/add 冲突,test.rs 为 modify/delete。需要 rebase 到最新 dev 并重新解决冲突。

  2. device_error_to_ax_error 等辅助函数重复axdevice/src/device.rs:112-162axvm/src/vm.rs:71-118 有完全相同的 4 个函数。建议将 device_error_to_ax_errorunexpected_device_response 作为 axdevicepub 辅助函数导出,axvm 直接复用,避免后续不同步。

  3. register_device() 未同步推进 next_device_id:public register_device() 可接受任意 DeviceId,但不推进 allocator 计数器。虽然当前工厂路径通过 ctx.alloc_device_id() 正确分配,但公共 API 允许外部调用者注册低 ID 设备导致后续 legacy 分配冲突。建议同步推进 next_device_id = max(next_device_id, id.raw() + 1),或在文档中明确约束 ID 来源。

Powered by mimo-v2.5-pro

Comment thread virtualization/axvm/src/vm.rs Outdated
}
}

fn device_error_to_ax_error(error: DeviceError) -> AxError {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此处的 4 个辅助函数(device_error_to_ax_errorunexpected_device_responsedispatch_device_readdispatch_device_write)与 axdevice/src/device.rs:112-162 完全重复。建议将前两个作为 axdevicepub 函数导出,axvm 直接 use axdevice::device_error_to_ax_error 复用。当前已有的 #1258 合入进一步加剧了维护风险,因为 dev 上的代码也在演化这些类型。

Comment thread virtualization/axdevice/src/device.rs Outdated
/// Unlike the legacy `add_*_dev` helpers, this does not add the device to
/// the old MMIO/PIO/SysReg lists. The device must declare its own bus
/// resources through [`DeviceOps::resources`].
pub fn register_device(&mut self, device: Rc<dyn DeviceOps>) -> DeviceResult<DeviceId> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

register_device() 可接受外部传入的任意 DeviceId,但不推进 next_device_id。如果外部调用者注册了 ID=0 的设备,后续 add_mmio_dev 等 legacy 路径的 alloc_device_id() 也会从 0 开始分配,导致 DuplicateDeviceId panic。建议添加:self.next_device_id = self.next_device_id.max(id.raw() + 1);

@ZR233 ZR233 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当前 head:60b18bb96fa32e326193197e9ccd3e0e8d80888c

我重新检查了新增的 linker-section factory registration 提交,以及当前未解决 review threads。这个 PR 目前仍然不能合入:

  1. 分支与最新 dev 仍是 DIRTYgit merge-tree origin/dev origin/pr/1322 显示 7 个冲突文件:virtualization/axdevice/src/device.rsvirtualization/axdevice/src/factory.rsvirtualization/axdevice/src/lib.rsvirtualization/axdevice/tests/test.rsvirtualization/axdevice_base/src/irq.rsvirtualization/axdevice_base/src/lib.rsvirtualization/axvm/src/vm.rs。其中 factory.rs/irq.rs 是 add/add,test.rs 是 dev 删除但本 PR 修改,和 dev 已合入的 #1258 device foundation 明显重叠,需要先 rebase/merge dev 并重新整理最终方案。
  2. 当前 head 仍未修复 AxVmDevices::register_device() 不推进 next_device_id 的问题。virtualization/axdevice/src/device.rs:497-498 仍然直接 self.registry.register_device(device);虽然 factory path 先走 ctx.alloc_device_id(),但 public API 仍允许外部注册低 id,之后 legacy add_*_dev 分配同 id 时会在 registry 注册阶段触发 DuplicateDeviceId panic。现有当前线程 PRRT_kwDORlre086LEAVs 应继续保持未解决。
  3. device_error_to_ax_error / unexpected_device_response / dispatch_device_read / dispatch_device_write 仍在 axdevice/src/device.rs:112-164axvm/src/vm.rs:71-119 重复。这个问题也已有当前线程 PRRT_kwDORlre086LEAVq,需要在 rebase 后一并抽到共享位置,避免后续两个调度路径语义漂移。
  4. 当前 head 没有 check runs/status checks。

本轮验证:git diff --check origin/dev...HEAD 通过;我检查了 DeviceFactoryRegister / register_device_factory!DeviceFactoryCatalog::from_linker/find_uniqueruntime.ld section symbols、VGicR factory 注册路径和相关测试。由于当前仍有合并冲突和上述阻塞问题,本轮没有继续跑完整 clippy/QEMU;修复并 rebase 后建议至少跑 cargo fmtcargo xtask clippy --package axdevicecargo xtask clippy --package axdevice_basecargo xtask clippy --package axvm,以及本 PR 新增的 aarch64-device-migration/qemu/smoke 流程。

@ZR233
ZR233 requested a review from yfblock June 21, 2026 04:09

@mai-team-app mai-team-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查总结

PR 内容:引入统一设备模型,新增 axdevice_base 基础抽象(DeviceOpsBusAccess/BusResponseResourceDeviceFactoryDeviceFactoryRegister)、DeviceRegistry 路由表与冲突检查、LegacyDeviceAdapter 适配器、linker-section factory 自注册机制,并将 VGicR 迁移为直接实现 DeviceOps 的原生设备,新增 GpptRedistributorFactory

设计思路清晰,职责分离合理:catalog 描述"可创建"、registry 描述"已注册",factory 返回 Vec<Rc<dyn DeviceOps>> 不暴露 BaseDeviceOps。linker-section 注册宏放在 axdevice_base,避免循环依赖,from_linker() 包含对齐和长度校验。VGicR 迁移完整,删除了旧 BaseDeviceOps impl 和 legacy_meta,unit test 覆盖面广(1000+ 行)。

CI 状态:组织侧 CI 未触发 check runs(来自 fork 的 PR),无法通过 CI 判断正确性。PR 作者声称本地 cargo xtask clippy --package axdevice/axdevice_basecargo test -p axdevicecargo xtask axvisor test qemu --arch aarch64 --test-group aarch64-device-migration --test-case smoke 均通过。

合并冲突(阻塞)mergeable_state: dirty。当前 HEAD 704891c 与最新 devafad7919)存在 7 个文件冲突

  • virtualization/axdevice/src/device.rs(content)
  • virtualization/axdevice/src/factory.rs(add/add)
  • virtualization/axdevice/src/lib.rs(content)
  • virtualization/axdevice/tests/test.rs(modify/delete:dev 删除了该文件)
  • virtualization/axdevice_base/src/irq.rs(add/add)
  • virtualization/axdevice_base/src/lib.rs(content)
  • virtualization/axvm/src/vm.rs(content)

dev 已合入 #1258 "Issue 595 device foundation",引入了 registration.rsfactory.rsirq.rs 等同类基础模块,与本 PR 高度重叠。需要 rebase/merge dev 并重新整理最终方案后才能合入。

register_device() 未同步推进 next_device_id(阻塞):见 inline comment。此问题已在 Copilot、ZR233 及前两轮 Mai review 中多次指出,当前 HEAD 仍未修复。

辅助函数重复(建议)device_error_to_ax_errorunexpected_device_responsedispatch_device_readdispatch_device_writeaxdevice/src/device.rs:112-164axvm/src/vm.rs:71-118 完全重复。见 inline comment。建议将前两个作为 axdevicepub 辅助函数导出,axvm 复用。

相关 PR#1320(已关闭的更激进 axbus 抽象层方案),本方案更务实。当前 open PR #1323(ptrace)、#1325(macOS selfbuild)与本 PR 无关。

总结:设计和实现方向正确,linker-section factory 机制和 VGicR 迁移已验证可行。但合并冲突和 register_device ID 同步问题是阻塞项,需要先解决。建议 rebase 后同时修复 ID 同步和辅助函数重复,然后重新跑 cargo fmt --checkcargo xtask clippy --package axdevice/axdevice_base/axvmcargo test -p axdevice/axdevice_base 和 QEMU smoke 测试。

Powered by mimo-v2.5-pro

Comment thread virtualization/axdevice/src/device.rs Outdated
/// the old MMIO/PIO/SysReg lists. The device must declare its own bus
/// resources through [`DeviceOps::resources`].
pub fn register_device(&mut self, device: Rc<dyn DeviceOps>) -> DeviceResult<DeviceId> {
self.registry.register_device(device)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

register_device() 只做 self.registry.register_device(device),没有推进 next_device_id 计数器。如果外部调用者注册了一个 DeviceId 较小的 native 设备(例如 factory 之外手动注册 ID=0),后续 legacy 路径 alloc_device_id() 也会从当前 next_device_id(仍为 0)开始分配,导致 DuplicateDeviceId panic。

建议在注册成功后同步推进计数器:

pub fn register_device(&mut self, device: Rc<dyn DeviceOps>) -> DeviceResult<DeviceId> {
    let id = self.registry.register_device(device)?;
    self.next_device_id = self.next_device_id.max(id.raw() + 1);
    Ok(id)
}

此问题已在 Copilot 和前两轮 review 中多次提出,当前 HEAD 仍未修复。

Comment thread virtualization/axdevice/src/device.rs Outdated
) {
let rw = if read { "read" } else { "write" };
trace!("emu_device {rw}: {addr_type} {addr:#x} in range {addr_range:#x} with width {width:?}")
fn device_error_to_ax_error(error: DeviceError) -> AxError {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

device_error_to_ax_errorunexpected_device_responsedispatch_device_readdispatch_device_write 四个函数与 axvm/src/vm.rs:71-118 完全重复。建议将 device_error_to_ax_errorunexpected_device_response 作为 axdevicepub 函数导出,axvm 直接 use axdevice::... 复用。后续 dev 分支持续演化这些类型,两处维护不同步的风险更高。

@mai-team-app mai-team-app Bot mentioned this pull request Jun 21, 2026

@ZR233 ZR233 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本轮复审 current head 704891c82a7c4c9c18badb2e2bc57ba415b8bf5e。新增的 linker-section factory 设计记录提交没有解决此前阻塞点,当前仍不能合入。

阻塞项:

  1. 分支仍与最新 dev 冲突。隔离 worktree 中执行 git merge --no-commit --no-ff origin/dev 后确认 7 个冲突文件:virtualization/axdevice/src/device.rsvirtualization/axdevice/src/factory.rs(add/add)、virtualization/axdevice/src/lib.rsvirtualization/axdevice/tests/test.rs(dev 删除、PR 修改)、virtualization/axdevice_base/src/irq.rs(add/add)、virtualization/axdevice_base/src/lib.rsvirtualization/axvm/src/vm.rs。当前 dev 已有 device foundation 相关 DeviceFactoryRegistry/factory/irq 基础设施,本 PR 需要 rebase 后重新整理最终方案。
  2. 当前未解决 review thread 仍成立:AxVmDevices::register_device() 仍只是 self.registry.register_device(device)virtualization/axdevice/src/device.rs:497-498),没有在手动注册成功后推进 next_device_id。如果外部注册了较小 DeviceId,后续 legacy add_*_dev 仍可能从旧计数器分配相同 id,并在 registry 阶段触发 DuplicateDeviceId
  3. device_error_to_ax_error / unexpected_device_response / dispatch_device_read / dispatch_device_write 仍在 virtualization/axdevice/src/device.rs:112-164virtualization/axvm/src/vm.rs:71-119 重复。这个问题不一定单独阻塞设计验证,但在 rebase 到已演进的 dev 后更容易出现语义漂移,建议一并抽到共享位置。
  4. 当前 head 没有可用 check runs;由于分支仍有冲突和上述阻塞,本轮未继续跑完整 clippy/QEMU。

已做检查:PR metadata/head、当前 review threads、CI 状态、git diff --check origin/dev...HEAD(通过)、register_device 和重复 dispatch helper 代码、与 origin/dev 的冲突 dry-run、以及 origin/dev 上已有的 device foundation/factory 相关代码。

重复/重叠分析:本 PR 与已经合入 dev 的 device foundation 方向明显重叠,且冲突文件正落在 factory/irq/device/vm 分发核心路径上;需要先 rebase 并明确采用/整合哪套基础抽象后再判断最终 diff。

@ZCShou ZCShou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本轮复审 current head 704891c82a7c4c9c18badb2e2bc57ba415b8bf5e,结论仍是需要修改后再评审。

这个 PR 的目标是把 AxVisor 设备分发统一到 registry/factory 模型:新增 axdevice_base 里的设备模型、bus/resource/irq/factory 抽象,在 axdevice 中加入 registry 和 legacy adapter,并把 AArch64 VGicR 迁移到 native DeviceOps 路径,同时补了一个 aarch64 QEMU smoke case。整体方向与项目正在收敛的 device foundation 是一致的,但当前分支已经落后于 dev,不能直接合入。

阻塞项:

  1. 分支仍与当前 dev 冲突。隔离 worktree 中用 git merge-tree --name-only origin/dev HEAD 复现了冲突,涉及 virtualization/arm_vgic/src/v3/vgicr.rsvirtualization/axdevice/src/device.rsvirtualization/axdevice/src/factory.rsvirtualization/axdevice/src/lib.rsvirtualization/axdevice/tests/test.rsvirtualization/axdevice_base/src/irq.rsvirtualization/axdevice_base/src/lib.rsvirtualization/axvm/src/vm.rs。其中 factory/irq/device/vm 分发路径正是本 PR 的核心改动,需要先 rebase/merge 最新 dev 并语义化整合,而不是按旧抽象继续叠加。
  2. 当前 dev 已经通过 #1258#1335 合入了同域 device foundation:DeviceFactoryRegistryDeviceRegistrationBusRouterDeviceRegistryIrqSink、adapter/registration 等基础设施都已经存在。本 PR 现在与 base 实现明显重叠,且冲突文件正落在这些接口上。需要在 rebase 后明确采用 base 的哪些抽象、保留本 PR 的哪些增量,否则最终合并会形成两套相近模型。
  3. 现有未解决 review thread 仍然成立:AxVmDevices::register_device() 仍只是 self.registry.register_device(device),没有在手动注册成功后推进 next_device_id。如果外部 native 设备注册较小 DeviceId,后续 legacy add_*_dev 仍可能分配同一个 id,并在 registry 注册阶段触发 DuplicateDeviceId。这个问题应继续保持 open。
  4. 另一个当前未解决 thread 也仍然成立:device_error_to_ax_error / unexpected_device_response / dispatch_device_read / dispatch_device_writeaxdevice/src/device.rsaxvm/src/vm.rs 仍有重复副本。单独看不是最严重的运行时问题,但在当前 base 已经演进这些类型的情况下,rebase 时应一并收敛到共享实现,避免后续语义漂移。

验证情况:

  • git diff --check origin/dev...HEAD 通过。
  • rg -n '\[patch\.crates-io\]' -g 'Cargo.toml' . 未发现 crates.io patch;本 PR 也没有 Cargo metadata 改动。
  • gh pr checks 1322 --repo rcore-os/tgoskits --watch=false 返回当前 head 没有 check runs。
  • 由于当前 head 与 dev 冲突且已被 base 的 device foundation 覆盖/重叠,本轮没有继续跑 clippy 或 QEMU smoke;在 rebase/整合后建议至少重新跑 cargo fmt --checkcargo xtask clippy --package axdevicecargo xtask clippy --package axdevice_basecargo xtask clippy --package axvm,以及 PR 新增的 cargo xtask axvisor test qemu --arch aarch64 --test-group aarch64-device-migration --test-case smoke

重复/重叠分析:open PR 搜索中除 #1322 外,命中项主要是 release PR 和较宽的 runtime 重构,未发现另一个仍打开的同意图 PR;真正的重叠来自当前 dev 已合入的 #1258/#1335。因此这个 PR 下一步应先 rebase 并重做最终 diff,再判断 VGicR factory 迁移和新增 smoke case 是否仍是独立增量。

未解决线程处理:上述两个当前未过期 thread 仍行为有效,本轮未 resolve;旧的 outdated thread 保持不动。现有行内评论已经覆盖同一问题,这里不重复新增相同 inline comment。

@baitwo02 baitwo02 closed this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants