Skip to content

nvme_driver: add timeout to controller ready/reset wait loops - #3976

Open
Novikov Kirill (knQzx) wants to merge 1 commit into
microsoft:mainfrom
knQzx:fix/nvme-driver-reset-timeout
Open

nvme_driver: add timeout to controller ready/reset wait loops#3976
Novikov Kirill (knQzx) wants to merge 1 commit into
microsoft:mainfrom
knQzx:fix/nvme-driver-reset-timeout

Conversation

@knQzx

Copy link
Copy Markdown
Contributor

the enable() wait-for-RDY loop and Bar0::reset() wait-for-RDY-clear loop both poll with Backoff forever, so a stuck emulated device, a broken physical device, or a malicious VF that never lets the controller reach the expected state hangs the driver thread, and it also blocks the fuzzer from exercising setup-time parameters

this is the minimal fix from the issue's suggested fix: derive a deadline from CAP.TO (500ms units, 3s floor when TO is 0) and bound both loops

  • ready loop bails with anyhow on expiry, matching the surrounding bail messages
  • reset loop returns Err(csts) on expiry, signature unchanged since callers only log it
  • unit test mocks a short CAP.TO and a stuck CSTS and checks NvmeDriver::new errors instead of hanging

kept out of scope on purpose: no full reset support or DMA quiescence, and nothing frees or unaliases DMA memory on the timeout path, that broader work is a bigger change and can follow up separately

fixes #3022

Copilot AI review requested due to automatic review settings July 20, 2026 07:15
@knQzx
Novikov Kirill (knQzx) requested review from a team as code owners July 20, 2026 07:15

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 hardens the NVMe driver’s controller bring-up/reset paths by adding a CAP.TO-derived deadline to previously-unbounded CSTS.RDY polling loops, preventing permanent hangs when a controller never reaches the expected ready state (including fuzzing scenarios).

Changes:

  • Add a ready_timeout(cap) helper derived from CAP.TO (500ms units, 3s floor when TO=0) and use it to bound the CC.EN=1 “wait for RDY set” loop.
  • Bound Bar0::reset()’s “wait for RDY clear” loop using the same CAP.TO-derived deadline (preserving the existing Result<(), u32> signature).
  • Add a unit test that mocks CAP.TO and a stuck CSTS to assert NvmeDriver::new() errors instead of hanging, and implement set_mock_response_u32 to support that mock.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs Adds a regression test for controller-ready timeout and enables mocking u32 MMIO reads (CSTS).
vm/devices/storage/disk_nvme/nvme_driver/src/registers.rs Introduces CAP.TO-based timeout helper and applies it to the reset RDY-clear polling loop.
vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs Applies CAP.TO-based timeout to the controller-enable RDY-set polling loop and bails with an error on expiry.

@knQzx

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

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.

nvme_driver: add timeout to controller ready/reset wait loops

2 participants