You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
gdma: return the device to its initial state on reset - #4932
GdmaDevice::reset is empty, though ChangeDeviceState::reset is documented to reset the device "to its initial state, for a fresh boot". After a VM reset with the HWC up:
the next boot's ESTABLISH_HWC fails with HwcAlreadyActive, so the guest driver can't bring the device up;
the HWC task and the vport data path keep running against the rings the last boot set up, in memory the new boot now owns;
every queue stays allocated.
reset now stops the HWC and the data path, frees the queues, and clears shared memory, a pending destroy and config space. The rx-disable path and reset share a new Vport::stop_data_path.
A vport also records whether its endpoint holds queues. The flag is set before get_queues is awaited, so an endpoint whose get_queues was cut short still gets stopped. stop_data_path clears the task, and that flag, only after the matching await returns, so a stop dropped partway (an RX disable cut short by a reset, say) is finished by the next stop, reset or RX enable. reset destructures the device, the bnic, each vport and the queue table, so a field added later has to be handled there before it compiles.
In net_mana, get_queues and stop now run their body in a task on the endpoint's spawner. A caller dropped mid call no longer abandons an HWC request the device is already working on: the task finishes it, and the next get_queues or stop waits for that task first. Before, the next call could destroy the same queue a second time, which the device rejects, or never learn about a queue that had been created.
GdmaDevice::stop is still empty, so the device keeps working while paused. That's a separate problem and I left it alone here.
Tests are unit tests against GdmaDevice and net_mana; the net_mana ones drop get_queues and stop at every point while their task runs, then require the next get_queues and stop to succeed. The reset comes from our downstream networking tree, where the data path runs one task per queue, and was tested there: Windows 11 guests (one with HVCI) and Linux guests with a MANA VF went through repeated guest reboots and hard resets with no crash and no missing device. The current version, including the cancelled-stop handling and the net_mana change, has since run the same reboot and hard-reset cycles there, plus that tree's network and pause/resume tests, with the same results as the earlier run. None of those runs is known to have cancelled a call partway, so that path is covered by the unit tests only. I haven't booted this branch itself. Re-tested on 2026-11-05, after our downstream tree caught up with main: the same reboot and hard-reset cycles, and pause/resume, passed again with a MANA VF.
GdmaDevice::reset was empty. After a VM reset the HWC task and the vport data
path kept running against the previous boot's rings, and every queue stayed
allocated. The next boot's ESTABLISH_HWC failed with HwcAlreadyActive.
Stop the HWC and the data path, free the queues, and clear the shared memory,
the pending destroy and config space. The rx-disable path and reset now share
Vport::stop_data_path.
This PR implements GDMA reset cleanup so a fresh guest boot can re-establish the hardware channel and allocate queues without retaining state from the previous boot. Pause behavior remains unchanged.
Changes:
Stop the hardware channel and vport data paths, free queues, and clear shared memory and PCI configuration.
Share data-path shutdown logic between reset and RX-disable.
Add unit tests for repeated channel establishment, register cleanup, and data-path teardown.
File
Description
vm/devices/net/gdma/src/queues.rs
Add cleanup for all allocated queues.
vm/devices/net/gdma/src/lib.rs
Implement device reset and add reset regression tests.
vm/devices/net/gdma/src/bnic.rs
Share data-path shutdown, reset vport state, and test teardown.
The reason will be displayed to describe this comment to others. Learn more.
In general, I prefer reset to be implemented via exhaustive destructuring so that we don't forget to reset new fields as they are added. This applies throughout. (Same for save/restore, btw.)
The reason will be displayed to describe this comment to others. Learn more.
Done in ec307a7: GdmaDevice, Queues, BasicNic and each Vport are reset by destructuring now. msix and regmap are bound to _, the way nvme does it: the config space reset covers the MSI-X capability, and the register map is fixed. The device has no save/restore yet (SavedStateNotSupported), so nothing to change there.
Reset now destructures the device, the bnic, each vport and the queue
table, so a field added later has to be handled before it compiles.
A vport tracks whether its endpoint still has queues out. Stopping the
data path clears the task and that flag only after each step finishes,
so a stop cancelled partway, for example an RX disable interrupted by a
reset, is completed by the next stop, reset, or RX enable instead of
leaving the endpoint with queues it never got back.
The emulated device runs the backend's get_queues and stop under
until_stopped, so either call can be dropped at any await. Three things
then went wrong in the driver stack underneath:
- mana_driver posted an HWC request and only consumed its two
completions inside the request future. A dropped request left them
pending, and the next request reused the request and response pages
while the device was still answering the old one, which wedged the
channel. The driver now records the posted request and finishes it
before the next request (or a save) reuses the pages.
- net_mana built the queue resources in a local arena, so a dropped
get_queues leaked them. A guard now hands them to the endpoint, and
the next get_queues or stop tears them down.
- stop took the whole arena before destroying it, so a dropped stop
leaked the remainder. ResourceArena::destroy_in_place removes each
resource only after its teardown returns.
Tests drop get_queues, stop and a single HWC request at every await
point against the emulated device and check that nothing is lost and
the next call finishes the work.
If create_eq is cancelled after posting its request, the device can create the EQ, but the arena never records its handle or removes the consumed DMA-region entry. Draining completions here discards that response, so later cleanup cannot disable the EQ before freeing its backing memory. Preserve the allocation through response processing and arena registration, or undo it before reusing the response page; the same issue affects DMA-region and WQ creation.
The vport only recorded that the endpoint needed a stop after
get_queues returned. A reset that cancelled the enable while the
endpoint was still allocating skipped endpoint.stop(), so whatever the
endpoint had allocated stayed behind. Mark the endpoint before the call
starts, so a cancelled or failed get_queues is stopped like a completed
one.
get_queues and stop issue several HWC requests, and their caller can
drop them at any await. Resuming a dropped call from the endpoint's
records is not enough: a request posted before the drop still runs on
the device, so the next call either repeats it (a second destroy of the
same queue, which the device rejects) or never records what it created.
Run the body of each call in a task on the endpoint's spawner instead.
The caller only waits for the task, so dropping the caller no longer
abandons a request. The next get_queues or stop first collects the
leftover task: queues nobody received are dropped and their resources
kept for the following stop. A pending task is detached, not cancelled,
when the endpoint is dropped.
This replaces the resumable arena and the HWC request drain from the
previous commit, which are no longer needed.
The tests drop get_queues and stop at every point while their task is
still running, then require a further get_queues and stop on the same
endpoint to succeed. That fails if a queue was created twice, destroyed
twice, or left behind.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The reset path coordinates HWC cancellation with hardware-backed DMA teardown, but this branch lacks boot/reset validation and downstream testing did not exercise stop cancellation, warranting human review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GdmaDevice::resetis empty, thoughChangeDeviceState::resetis documented to reset the device "to its initial state, for a fresh boot". After a VM reset with the HWC up:ESTABLISH_HWCfails withHwcAlreadyActive, so the guest driver can't bring the device up;resetnow stops the HWC and the data path, frees the queues, and clears shared memory, a pending destroy and config space. The rx-disable path andresetshare a newVport::stop_data_path.A vport also records whether its endpoint holds queues. The flag is set before
get_queuesis awaited, so an endpoint whoseget_queueswas cut short still gets stopped.stop_data_pathclears the task, and that flag, only after the matching await returns, so a stop dropped partway (an RX disable cut short by a reset, say) is finished by the next stop, reset or RX enable.resetdestructures the device, the bnic, each vport and the queue table, so a field added later has to be handled there before it compiles.In
net_mana,get_queuesandstopnow run their body in a task on the endpoint's spawner. A caller dropped mid call no longer abandons an HWC request the device is already working on: the task finishes it, and the nextget_queuesorstopwaits for that task first. Before, the next call could destroy the same queue a second time, which the device rejects, or never learn about a queue that had been created.GdmaDevice::stopis still empty, so the device keeps working while paused. That's a separate problem and I left it alone here.Tests are unit tests against
GdmaDeviceandnet_mana; thenet_manaones dropget_queuesandstopat every point while their task runs, then require the nextget_queuesandstopto succeed. The reset comes from our downstream networking tree, where the data path runs one task per queue, and was tested there: Windows 11 guests (one with HVCI) and Linux guests with a MANA VF went through repeated guest reboots and hard resets with no crash and no missing device. The current version, including the cancelled-stop handling and thenet_manachange, has since run the same reboot and hard-reset cycles there, plus that tree's network and pause/resume tests, with the same results as the earlier run. None of those runs is known to have cancelled a call partway, so that path is covered by the unit tests only. I haven't booted this branch itself. Re-tested on 2026-11-05, after our downstream tree caught up with main: the same reboot and hard-reset cycles, and pause/resume, passed again with a MANA VF.