fix(ahci): integrate controller with PCI driver model - #2168
Conversation
Register AHCI controllers through the standard PCI driver lifecycle and distinguish absent controllers, empty ports, and genuine device failures. Preserve PCI command state and make controller, port, command-memory, and disk ownership explicit during probe, I/O, removal, and shutdown. Add bounded physical-page allocation for DMA masks, reject incompatible pooled buffers, and provide an on-demand debugfs self-test with deterministic buddy split, merge, fragmentation, and address-bound checks. Align block-device publication with Linux semantics by retaining the whole-disk node across partition scan or publication failures, preserving MBR slot numbers, and clipping partitions to device capacity. Add dunitest coverage for DMA allocation behavior and zero-capacity loop devices. The change has been validated with kernel builds, targeted dunitests, and QEMU AHCI matrices covering empty, valid-MBR, invalid-MBR, and injected-I/O-error devices. Signed-off-by: longjin <longjin@dragonos.org>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6628f3874e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Treat synchronization as a successful no-op only when IDENTIFY reports neither an enabled write cache nor a supported FLUSH CACHE command. This prevents completed writes from being reported as unsupported while preserving real command failures. Track the advertised reliable-flush capability separately from the command selected for synchronization. If write cache is enabled but FLUSH capability bits are absent, follow Linux libata and attempt the base FLUSH CACHE command without advertising a reliable power-loss barrier. Signed-off-by: longjin <longjin@dragonos.org>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a395bbcef8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Make PCI removal a non-fallible terminal notification and detach AHCI block devices even while stale userspace or mount references remain. Gate new I/O, stop hardware when accessible, disable bus mastering, and keep delayed drops hardware-silent. Retain DMA allocations when engine shutdown cannot be proven, then reclaim the BDF-keyed quarantine only after a later probe completes an HBA reset with bus mastering disabled. Add irreversible best-effort block-device unpublication for physically detached hardware. Validate the complete 48-bit IDENTIFY capacity, avoid yielding on the first polling iteration, and train candidate links concurrently in one bounded controller window. Stop provisional FIS receivers without invalidating successful link classification. Validated with make kernel, x86_64 workspace tests, QEMU empty-controller boot, and QEMU AHCI disk discovery/read/sync smoke tests. Signed-off-by: longjin <longjin@dragonos.org>
|
@codex review |
Match the repository rustfmt output for the non-short-circuit provisional port stop expression. FMT_CHECK=1 make fmt now passes, including the kernel clippy check. Signed-off-by: longjin <longjin@dragonos.org>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 941dbbbba2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stop all implemented ports in two controller-wide phases: clear ST and wait up to 500 ms for every CR bit, then clear FRE and give every FR bit a separate 500 ms window. Each scan visits every port, so teardown latency no longer scales with port count. Treat an un-stoppable port during multi-port probe as a controller-fatal rollback. Quiesce published I/O, stop or reset the HBA, enter the detached terminal state, retire or quarantine DMA, and unpublish earlier disks before releasing the BDF probe reservation. Reuse the final Bus Master disable, detached publication, and DMA retirement sequence between normal remove and probe rollback so delayed mount references remain hardware-silent. Validated with make kernel, FMT_CHECK=1 make fmt, git diff --check, QEMU AHCI boot/device discovery/sync, and independent architecture, safety, and semantic reviews. Signed-off-by: longjin <longjin@dragonos.org>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4b9ef4506
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.accepting_io.store(false, Ordering::Release); | ||
| let command = self.pci.status_command().1; | ||
| self.pci.set_command(command & !Command::BUS_MASTER); | ||
| self.quarantined_dma.lock().push(buffer); |
There was a problem hiding this comment.
当一个端口的命令失败且该端口也无法停止、同时其他端口仍有命令在途时，这里会立即清除控制器级 Bus Master，却只隔离当前请求的 DMA 缓冲区。其他端口随后可能产生 HBFS/HBDS/IFS 等主机总线错误，但 wait_slot() 只检查 TFES，并会在 CI 清零时返回成功，因此读取可能复制未完成的 DMA 数据，写入也可能被误报为成功；应先让所有在途请求进入明确失败状态并保留其缓冲区，再关闭全局 DMA。
Useful? React with 👍 / 👎.
| for (port_no, _, _, port_type) in link_states { | ||
| if port_type != Some(HbaPortType::Sata) { | ||
| continue; | ||
| } | ||
| if let Err(err) = controller.publish_port(port_no) { |
There was a problem hiding this comment.
当多个端口都已被分类为 SATA、但不完成 IDENTIFY 且超时后的端口停止能够成功时，这个循环会串行调用 publish_port()，而每次 IDENTIFY 都有独立的 30 秒 wait_slot() 超时。32 端口控制器因此可让同步 PCI probe 和系统启动阻塞约 16 分钟；此前共享的链路训练窗口并不覆盖此阶段，应并发推进 IDENTIFY 或共享一个控制器级 deadline。
Useful? React with 👍 / 👎.
| let mut first_error = None; | ||
| for disk in disks { | ||
| if let Err(err) = disk.flush_for_teardown() { |
There was a problem hiding this comment.
在 remove 或 shutdown 期间，如果多个磁盘均不完成 FLUSH CACHE、但各端口之后仍可停止，这个串行循环会为每块盘分别等待最多 30 秒，32 盘控制器可在进入已有总超时保护的停止阶段之前挂起约 16 分钟。应让 teardown 刷新共享总 deadline，或并发提交并轮询所有端口的刷新命令。
Useful? React with 👍 / 👎.
1 participant
Summary
Root cause
The previous AHCI path mixed PCI discovery, controller initialization, disk construction, partition probing, and block-device publication in one startup-time flow. An empty AHCI port could therefore reach disk I/O and surface
EIOas if driver initialization had failed. The same path also lacked a reliable physical-address bound for DMA allocations and had ambiguous ownership during failure and removal.Behavior after this change
Validation
make kernelgit diff --checkDUNITEST_PATTERN=dma_allocator_selftest make test-dunit— 2 tests passedDUNITEST_PATTERN=loop_semantics make test-dunit— 7 tests passedEIOScope
This change intentionally does not add IRQ-driven completion, NCQ, hotplug, or full runtime error recovery. Those are separate performance and feature projects and are not required to correct empty-controller discovery and ownership semantics.