Skip to content

fix(vmm): close cold-boot BSP/AP race on dual-socket NUMA hosts#5897

Open
JackThomson2 wants to merge 5 commits into
firecracker-microvm:mainfrom
JackThomson2:fix/cold_boot_smp
Open

fix(vmm): close cold-boot BSP/AP race on dual-socket NUMA hosts#5897
JackThomson2 wants to merge 5 commits into
firecracker-microvm:mainfrom
JackThomson2:fix/cold_boot_smp

Conversation

@JackThomson2

@JackThomson2 JackThomson2 commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md.
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

@codecov

codecov Bot commented May 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.17241% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.04%. Comparing base (9e8b921) to head (4e4ea18).

Files with missing lines Patch % Lines
src/vmm/src/vstate/vm.rs 72.46% 38 Missing ⚠️
src/vmm/src/builder.rs 65.21% 24 Missing ⚠️
src/vmm/src/vstate/vcpu.rs 87.50% 9 Missing ⚠️
src/vmm/src/arch/x86_64/vcpu.rs 85.71% 1 Missing ⚠️
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     
Flag Coverage Δ
5.10-m5n.metal 83.36% <75.17%> (+0.05%) ⬆️
5.10-m6a.metal 82.72% <75.17%> (+0.06%) ⬆️
5.10-m6g.metal 80.01% <74.91%> (+0.07%) ⬆️
5.10-m6i.metal 83.36% <75.17%> (+0.05%) ⬆️
5.10-m7a.metal-48xl 82.71% <75.17%> (+0.06%) ⬆️
5.10-m7g.metal 80.01% <74.91%> (+0.07%) ⬆️
5.10-m7i.metal-24xl 83.33% <75.17%> (+0.05%) ⬆️
5.10-m7i.metal-48xl 83.34% <75.17%> (+0.05%) ⬆️
5.10-m8g.metal-24xl 80.01% <74.91%> (+0.06%) ⬆️
5.10-m8g.metal-48xl 80.01% <74.91%> (+0.07%) ⬆️
5.10-m8i.metal-48xl 83.34% <75.17%> (+0.05%) ⬆️
5.10-m8i.metal-96xl 83.34% <75.17%> (+0.05%) ⬆️
6.1-m5n.metal 83.39% <75.17%> (+0.05%) ⬆️
6.1-m6a.metal 82.74% <75.17%> (+0.06%) ⬆️
6.1-m6g.metal 80.01% <74.91%> (+0.06%) ⬆️
6.1-m6i.metal 83.38% <75.17%> (+0.05%) ⬆️
6.1-m7a.metal-48xl 82.73% <75.17%> (+0.06%) ⬆️
6.1-m7g.metal 80.01% <74.91%> (+0.06%) ⬆️
6.1-m7i.metal-24xl 83.40% <75.17%> (+0.05%) ⬆️
6.1-m7i.metal-48xl 83.40% <75.17%> (+0.05%) ⬆️
6.1-m8g.metal-24xl 80.00% <74.91%> (+0.06%) ⬆️
6.1-m8g.metal-48xl 80.01% <74.91%> (+0.07%) ⬆️
6.1-m8i.metal-48xl 83.41% <75.17%> (+0.06%) ⬆️
6.1-m8i.metal-96xl 83.40% <75.17%> (+0.05%) ⬆️
6.18-m5n.metal 83.39% <75.17%> (+0.05%) ⬆️
6.18-m6a.metal 82.74% <75.17%> (+0.06%) ⬆️
6.18-m6g.metal 80.01% <74.91%> (+0.06%) ⬆️
6.18-m6i.metal 83.38% <75.17%> (+0.05%) ⬆️
6.18-m7a.metal-48xl 82.73% <75.17%> (+0.06%) ⬆️
6.18-m7g.metal 80.00% <74.91%> (+0.06%) ⬆️
6.18-m7i.metal-24xl 83.40% <75.17%> (+0.05%) ⬆️
6.18-m7i.metal-48xl 83.40% <75.17%> (+0.06%) ⬆️
6.18-m8g.metal-24xl 80.01% <74.91%> (+0.06%) ⬆️
6.18-m8g.metal-48xl 80.01% <74.91%> (+0.06%) ⬆️
6.18-m8i.metal-48xl 83.40% <75.17%> (+0.05%) ⬆️
6.18-m8i.metal-96xl 83.40% <75.17%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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>
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.

1 participant