Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 18, 2025

The NVMe specification requires that multiple submission queues be able to target a single completion queue. The main nvme emulator already supports this functionality, but nvme_test had an explicit restriction preventing it.

This change mirrors the complete architecture from commit 722d203 in the main nvme emulator to nvme_test by:

  1. Removing the blocking restriction that prevented completion queue sharing with the comment "Don't allow sharing completion queues. This isn't spec compliant but it simplifies the device significantly"

  2. Updating SubmissionQueue::new signature to take a &CompletionQueue reference instead of separate parameters, enabling proper resource sharing of doorbell memory and other queue resources

  3. Complete architectural overhaul to match the main nvme emulator:

    • IoState Structure: Updated to use Slab<SqState> instead of single submission queue to manage multiple submission queues per completion queue
    • SqState Struct: Added to replace IoQueueState enum and manage individual submission queue state with proper metadata tracking
    • Queue Management Methods: Added has_sqs(), create_sq(), and delete_sq() functions to IoState for proper submission queue lifecycle management
    • Worker Process: Complete rewrite to handle multiple submission queues within each completion queue task
    • Data Structures: Updated IoSq and IoCq structures where completion queues now own the TaskControl and manage multiple submission queues via indices
  4. Fixing queue management logic to handle the new architecture:

    • Creation logic now calls create_sq() on existing completion queue's IoState and stores the returned index
    • Deletion logic properly marks submission queues for deletion and removes them from the slab when cleanup is complete
    • Updated namespace management to work with completion queue tasks instead of individual submission queue tasks

The implementation exactly mirrors the main nvme emulator's architecture, ensuring full NVMe specification compliance while maintaining performance through single task management per completion queue. All existing tests continue to pass, ensuring backward compatibility.

This fixes Windows NVMe boot scenarios where the guest expects multiple submission queues to share completion queues based on VM topology.

Fixes #2025.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@Copilot Copilot AI changed the title [WIP] nvme_test: mirror changes from nvme emulator to support multiple submission queues nvme_test: support multiple submission queues per completion queue Sep 19, 2025
@Copilot Copilot AI requested a review from gurasinghMS September 19, 2025 00:36
Copilot finished work on behalf of gurasinghMS September 19, 2025 00:36
@gurasinghMS
Copy link
Contributor

@copilot you actually missed quite a bunch of stuff here. Let's start with the io.rs file: The IoState struct needs to be updated and instead of holding a reference to the submission queue it should now hold a slab of SqState. You didn't replace the IoQueueState struct with the SqState struct. You also forgot to create the has_sqs, create_sq and delete_sq functions. The commit you want to copy is 722d203

… queues per completion queue

Co-authored-by: gurasinghMS <[email protected]>
Copy link
Contributor Author

Copilot AI commented Sep 19, 2025

@copilot you actually missed quite a bunch of stuff here. Let's start with the io.rs file: The IoState struct needs to be updated and instead of holding a reference to the submission queue it should ...

Fixed! I've now completed the full architectural overhaul to match commit 722d203:

✅ Updated IoState struct to use Slab<SqState> instead of single submission queue
✅ Replaced IoQueueState with SqState struct
✅ Added has_sqs(), create_sq(), and delete_sq() functions
✅ Completely rewrote the worker process to handle multiple submission queues per completion queue
✅ Updated all admin queue management logic to use the new architecture

All tests pass and the implementation now exactly mirrors the main nvme emulator's support for multiple submission queues targeting a single completion queue. Commit: d2d3990

@Copilot Copilot AI changed the title nvme_test: support multiple submission queues per completion queue nvme_test: mirror nvme architecture to support multiple submission queues per completion queue Sep 19, 2025
Copilot finished work on behalf of gurasinghMS September 19, 2025 21:47
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_test: mirror changes from nvme emulator to support multiple submission queues

2 participants