Skip to content

[virtio-pmem] Implementation #5463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

[virtio-pmem] Implementation #5463

wants to merge 14 commits into from

Conversation

ShadowCurse
Copy link
Contributor

Changes

Add virtio-pmem device support.

Closes #5448

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.

@ShadowCurse ShadowCurse self-assigned this Oct 3, 2025
@codecov Codecov
Copy link

codecov bot commented Oct 3, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 73.86364% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.64%. Comparing base (ae7b046) to head (1f68a08).

Files with missing lines Patch % Lines
src/vmm/src/devices/virtio/pmem/device.rs 68.24% 67 Missing ⚠️
src/vmm/src/devices/virtio/pmem/event_handler.rs 23.07% 40 Missing ⚠️
src/vmm/src/builder.rs 77.14% 8 Missing ⚠️
src/vmm/src/rpc_interface.rs 0.00% 8 Missing ⚠️
src/firecracker/src/api_server/request/pmem.rs 77.27% 5 Missing ⚠️
src/vmm/src/devices/virtio/pmem/metrics.rs 85.29% 5 Missing ⚠️
src/vmm/src/vmm_config/pmem.rs 93.02% 3 Missing ⚠️
src/vmm/src/device_manager/persist.rs 96.55% 1 Missing ⚠️
src/vmm/src/devices/virtio/pmem/persist.rs 95.23% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #5463      +/-   ##
==========================================
- Coverage   82.79%   82.64%   -0.16%     
==========================================
  Files         263      269       +6     
  Lines       27218    27731     +513     
==========================================
+ Hits        22535    22917     +382     
- Misses       4683     4814     +131
Flag Coverage Δ
5.10-m5n.metal 82.79% <73.86%> (-0.19%) ⬇️
5.10-m6a.metal 82.05% <73.86%> (-0.18%) ⬇️
5.10-m6g.metal 79.45% <73.86%> (-0.13%) ⬇️
5.10-m6i.metal 82.78% <73.86%> (-0.20%) ⬇️
5.10-m7a.metal-48xl 82.03% <73.86%> (-0.18%) ⬇️
5.10-m7g.metal 79.46% <73.86%> (-0.12%) ⬇️
5.10-m7i.metal-24xl 82.75% <73.86%> (-0.20%) ⬇️
5.10-m7i.metal-48xl 82.76% <73.86%> (-0.19%) ⬇️
5.10-m8g.metal-24xl 79.45% <73.86%> (-0.12%) ⬇️
5.10-m8g.metal-48xl 79.46% <73.86%> (-0.12%) ⬇️
6.1-m5n.metal 82.81% <73.86%> (?)
6.1-m6a.metal 82.07% <73.86%> (-0.19%) ⬇️
6.1-m6g.metal 79.45% <73.86%> (-0.13%) ⬇️
6.1-m6i.metal 82.81% <73.86%> (-0.20%) ⬇️
6.1-m7a.metal-48xl 82.06% <73.86%> (-0.18%) ⬇️
6.1-m7g.metal 79.45% <73.86%> (-0.13%) ⬇️
6.1-m7i.metal-24xl 82.82% <73.86%> (-0.20%) ⬇️
6.1-m7i.metal-48xl 82.83% <73.86%> (-0.20%) ⬇️
6.1-m8g.metal-24xl 79.45% <73.86%> (-0.12%) ⬇️
6.1-m8g.metal-48xl 79.45% <73.86%> (-0.12%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

@ShadowCurse ShadowCurse force-pushed the virtio_pmem branch 2 times, most recently from d8f9547 to 5970613 Compare October 6, 2025 11:01
@ShadowCurse ShadowCurse marked this pull request as ready for review October 6, 2025 12:57
@ShadowCurse ShadowCurse added Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Enhancement Indicates new feature requests labels Oct 6, 2025
@ShadowCurse ShadowCurse force-pushed the virtio_pmem branch 2 times, most recently from a8bedbb to 1d2aeb2 Compare October 6, 2025 14:18
Manciukic
Manciukic reviewed Oct 7, 2025
View reviewed changes
Copy link
Contributor

@Manciukic Manciukic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • we should update docs/device-api.md.
  • changelog entry
  • any performance tests? we could check how fast we can read or write the entire pmem or maybe we can integrate it with the block tests using fio

@ShadowCurse ShadowCurse force-pushed the virtio_pmem branch 2 times, most recently from 9a554b4 to 4b4779b Compare October 7, 2025 12:59
bchalios
bchalios requested changes Oct 7, 2025
View reviewed changes
Copy link
Contributor

@bchalios bchalios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went half way through. Here's an initial set of comments

@ShadowCurse ShadowCurse force-pushed the virtio_pmem branch 10 times, most recently from 7eff29f to 4a19190 Compare October 8, 2025 12:19
@ShadowCurse ShadowCurse force-pushed the virtio_pmem branch 10 times, most recently from 5bac831 to 552e3a7 Compare October 13, 2025 08:50
@ShadowCurse
chore(virtio-pmem): add msync syscall to seccomp filters
fbc75a8 
msync is used by virtio-pmem device to trigger sync of mmaped
file content to the underlying file.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): add device configs and implementation
f5064d8 
Add implementations of device, event handling, metrics.
Add device config and builder types for API use.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): add device to the VmResources
d389be2 
Update VmResources type with virtio-pmem configuration
field to allow virtio-pmem devices be configured
through config files and later through API calls.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat: add a check for a single root device
c68fca3 
Both virtio-block and virtio-pmem can act as root devices
for a VM. Add a check to prevent specifing more than 1 root
device for a VM.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): add API requests parsing to Firecracker
e8ecf1a 
Add /pmem/id PUT request for virtio-pmem configuration.
Add corresponding metrics.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat: add allocator for past 64bit memory region
f858492 
Virtio-pmem devices need to allocate a memory region in guest physical
memory. The safe place to do this is past 64bit MMIO region.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat: add a counter for KVM slots
b5c2315 
Add a counter for KVM slot ids into VmCommon struct. This is done
because virtio-pmem device needs to obtain it's KVM slot id
independently from number of slots in GuestMemoryMmap.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): add device to the Vmm
05742fa 
Add methods to attach virtio-pmem devices to Vmm.
Add methods to create KVM memory slot for virtio-pmem devices.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): add snapshot support
a641734 
Add logic to store and restore virtio-pmem device information
in a snapshot.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): add integration tests
3c44fab 
Add functional and API tests for virtio-pmem device
and its configuration fields

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
feat(virtio-pmem): export device metrics
1c81caa 
Expose virtio-pmem metrics in the logger, so they are exported in
metrics.json.
Update integration tests to expect new metrics.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
chore(virtio-pmem): document new APIs
a9190a0 
Add description of pmem APIs in swagger file and
device-api.md

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
bchalios
bchalios previously approved these changes Oct 13, 2025
View reviewed changes
Manciukic
Manciukic previously approved these changes Oct 13, 2025
View reviewed changes
Copy link
Contributor

@Manciukic Manciukic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just 2 more typos, sorry if I missed them before!

docs/pmem.md Outdated

## Performance

Event thought `virtio-pmem` allows for the direct access of host pages from the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: /Event thought/Even though/s

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still an extra "t" left there

@ShadowCurse
doc(virtio-pmem): add documentation
878fed4 
Add new document about virtio-pmem configuration and usage.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse
chore: update CHANGELOG
1f68a08 
Add a note about addition of virtio-pmem device.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse ShadowCurse dismissed stale reviews from Manciukic and bchalios via 1f68a08 October 13, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Manciukic Manciukic Manciukic left review comments

@bchalios bchalios bchalios left review comments

@xmarcalx xmarcalx Awaiting requested review from xmarcalx xmarcalx is a code owner

@kalyazin kalyazin Awaiting requested review from kalyazin kalyazin is a code owner

@pb8o pb8o Awaiting requested review from pb8o pb8o is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@ShadowCurse ShadowCurse

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Enhancement Indicates new feature requests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[virtio-pmem] Add support for virtio-pmem device

3 participants

@ShadowCurse @bchalios @Manciukic