fix(vmm): close cold-boot BSP/AP race on dual-socket NUMA hosts#5897
Open
JackThomson2 wants to merge 5 commits into
Open
fix(vmm): close cold-boot BSP/AP race on dual-socket NUMA hosts#5897JackThomson2 wants to merge 5 commits into
JackThomson2 wants to merge 5 commits into
Conversation
a63e73c to
c3a7c46
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5897 +/- ##
==========================================
+ Coverage 83.00% 83.04% +0.03%
==========================================
Files 277 277
Lines 30106 30315 +209
==========================================
+ Hits 24989 25174 +185
- Misses 5117 5141 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Set the BSP to RUNNABLE and APs to INIT_RECEIVED during x86_64 vCPU configuration. This mirrors the expected wait-for-SIPI startup state and avoids KVM's UNINITIALIZED short-circuit before APs process INIT/SIPI on their first KVM_RUN. Refs: firecracker-microvm#5744 Signed-off-by: Jack Thomson <jackabt@amazon.com>
Allow vCPU threads to choose whether they start paused or enter KVM_RUN directly. Existing boot, snapshot, test, and CPU template helper call sites keep using the Paused state, so this only prepares the startup path for a later cold-boot change. Signed-off-by: Jack Thomson <jackabt@amazon.com>
Start cold-boot APs in a gated Running state and keep the BSP paused until final VMM setup is complete. The VMM installs its exit-event subscriber and seccomp filter before releasing APs and resuming the BSP, so guest code cannot run before the VMM can observe vCPU exits. Snapshot restore and GDB attach-at-boot continue to use the Paused entry path. The explicit VcpuStartState and must-use launch guard make the parked-vCPU handoff harder to misuse. Refs: firecracker-microvm#5744 Signed-off-by: Jack Thomson <jackabt@amazon.com>
Add unit coverage for the cold-boot run gate release and abort paths. The tests assert that waiters remain pending until release and that abort unblocks waiters without counting them as released. Signed-off-by: Jack Thomson <jackabt@amazon.com>
ca3c633 to
19975b2
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes
With the current approach of vCPU initialisation it leaves a potential race condition
in which the BP enters the kernel and attempts to init AP's before they are initialised
and ready to start up.
Update the flow so that AP's are started into vCPU run before start BP to eliminate
this race condition.
Reason
Fixing #5744
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.