verify add_used operation using guestmemory stubs #346
Conversation
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
|
@roypat Hi Patrick, thanks |
priyasiddharth
force-pushed
the
verify_add_used
branch
from
959d811 to
7113f45
Compare
priyasiddharth
commented
Jun 19, 2025
MatiasVara
reviewed
Jun 26, 2025
Add the verify_spec_2_7_7_2() proof to verify that the implementation of queue satisfies 2.7.7.2 requirement. The proof relies on whether the EVENT_IDX feature has been negotiated. Conversely with `test_needs_notification()` test, this proof `tests` for all possible values of the queue structure. Signed-off-by: Matias Ezequiel Vara Larsen <[email protected]> Signed-off-by: Siddharth Priya <[email protected]>
Add the verify_spec_2_7_7_2() proof to verify that the implementation of queue satisfies 2.7.7.2 requirement. The proof relies on whether the EVENT_IDX feature has been negotiated. Conversely with `test_needs_notification()` test, this proof `tests` for all possible values of the queue structure. Signed-off-by: Matias Ezequiel Vara Larsen <[email protected]>
GuestMemory trait brings in Bytes trait and therefore hardcodes impl for write_obj and store methods. This does not work well with Kani which now has to process these methods symbolically. The solution is to provide NOP impl for these methods. My current solution is to use kani stub macro to replace these methods by local stubs. Another solution can be to provide the proof writer a way to construct GuestMemory such that they can stub out methods as needed. Signed-off-by: Siddharth Priya <[email protected]>
priyasiddharth
force-pushed
the
verify_add_used
branch
3 times, most recently
from
25784f9 to
96d3228
Compare
Signed-off-by: Siddharth Priya <[email protected]>
priyasiddharth
force-pushed
the
verify_add_used
branch
from
96d3228 to
89e9614
Compare
MatiasVara
reviewed
Jul 31, 2025
| } else { | ||
| assert_eq!(queue.next_used, used_idx); | ||
|
|
||
| // Ideally, here we would want to actually read the relevant values from memory and |
There was a problem hiding this comment.
I think this comment is true in firecraker but is it still valid in the rust-vmm implementation?
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.
Summary of the PR
We need to stub out or provide fake implementation for
GuestMemoryread/write operations since they are too expensive forkanito execute as-is.This problem was faces by
firekrackeralso and solved by using a fake impl (link).in
vmm-virtioGuestMemory is hardcoded to use the default impl (link) so it is not easy to fake it. There are a few options i see.Bytestrait forGuestMemoryfor kani - needs changes tovm-memorycratevm-virtiofunctions useGuestMemoryin production andFakeGuestMemoryin kani - needs changes tovm-virtioto conditionally compile and use the rightGuestMemoryThis PR provides the third option. However, 1 or 2 maybe a better engineering solution. Looking for inputs.
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.