Use wrapper types to encode Vm & VmPages state #98
Conversation
This file contains 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
abrestic-rivos
force-pushed
the
topic/vm-state-cleanup
branch
from
1d26fa8 to
64a2053
Compare
|
I will take a second look, but it looks good from an initial pass. |
abrestic-rivos
force-pushed
the
topic/vm-state-cleanup
branch
from
64a2053 to
491b951
Compare
dgreid
approved these changes
Sep 16, 2022
PCI BAR memory will eventually need special treatment when compared to ordinary confidential memory, e.g. when assigning a device to a TVM, so give it its own region type. For now though it'll enable us to add some type-safety to VmPagesMapper. Signed-off-by: Andrew Bresticker <[email protected]>
Currently, once a VmPagesMapper is constructed, one can use any sort of page with it regardless of the region type it was constructed in. Make it type-safe by adding a type parameter indicating which type of mapper it is and by having map_page() for that type only support the proper PhysPage. Signed-off-by: Andrew Bresticker <[email protected]>
Avoid this use of deref polymorphism and remove the Deref implementation. We only used this ability inside of ActiveVmPages anyway. Signed-off-by: Andrew Bresticker <[email protected]>
Instead of including the state as a type parameter for the VmPages struct itself, introduce a wrapper VmPagesRef type which exposes the appropriate functionality for the VmPages depending on the VM's state. This allows us to transition between states without consuming (copying) the entire struct. Signed-off-by: Andrew Bresticker <[email protected]>
Similar to VmPages, remove the state type argument from Vm and introduce a VmRef type that serves as a reference to a Vm in a particular state. The various VmRef types are then used to implement the appropriate functionality for a VM in that state. This immediately enables a couple of small cleanups: finalize() no longer needs to consume the Vm struct and we can now implement drop() for Vm. Signed-off-by: Andrew Bresticker <[email protected]>
abrestic-rivos
force-pushed
the
topic/vm-state-cleanup
branch
from
491b951 to
9699abf
Compare
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.
While encoding the state of the VM as a type parameter in
VmandVmPagescreates clean separation between the APIs in each state, it means that transitions between states require consuming (and as a result, copying) these large structs. Introduce wrapperVmRef/VmPagesReftypes which expose the appropriate functionality for the current state of the VM instead, which still gets us the nice API separation with an extra layer of indirection. This is a pre-requisite to #65, and will enable us to do in-place initialization of these structs.Patches 1-3 are small cleanups which enable patch 4 (wrapper state types for
VmPages) and patch 5 (wrapper state types forVm).