Rename GuestMemory->GuestMemoryBackend, IoMemory->GuestMemory #6
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
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `1311bfa` to `ae7db2d`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@1311bfa...ae7db2d) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Andreea Florescu <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `ae7db2d` to `f67ef4c`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@ae7db2d...f67ef4c) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `f67ef4c` to `68d4dbf`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@f67ef4c...68d4dbf) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Added `+ ?Sized` to the requirements of the `Bytes ` auto implementation for `T: GuestMemory`, as otherwise an unnecessary `Sized` bound was inferred. Signed-off-by: Alexandru Agache <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `68d4dbf` to `7931077`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@68d4dbf...7931077) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
For the build_raw() alternative of the MmapRegion, the implementation was validating that the length of the mapping was a multiple of the page size. This generally makes sense, since the OS will round up to the nearest multiple anyway and only allocate full pages. There is one corner case though where this needs to be relaxed: when mapping a file with a length not divisible by the pagesize. According to the POSIX reference, the length will be rounded up and the extra memory zero-ed. Memory accesses will work as expected, even beyond the actual file length (but within the allocated memory), but any writes will not be persisted to the filesystem and the backing device. This is a use case where mapping an "uneven" length makes sense, so that such "out-of-bounds" checks can be caught by accesses via vm-memory primitives. This commit removes this constraint. Signed-off-by: alindima <[email protected]>
Signed-off-by: alindima <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `7931077` to `b037be3`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@7931077...b037be3) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Andreea Florescu <[email protected]>
serde_cbor is an unmaintained dependency used by criterion. Because criterion is only used in benchmarks, we can ignore for now this vulnerability. Signed-off-by: Laura Loghin <[email protected]>
Signed-off-by: Julian Stecklina <[email protected]>
Use v15 container image with Rust 1.58.1 Signed-off-by: Sergii Glushchenko <[email protected]>
Signed-off-by: Sergii Glushchenko <[email protected]>
To keep consistent with the standard library, return ok(0) instead of out bound error when the slice passed as a parameter for read/write is empty. Signed-off-by: Andreea Florescu <[email protected]> Co-author: [email protected]
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `d216a46` to `99fe2eb`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@d216a46...99fe2eb) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Laura Loghin <[email protected]>
Currently mmap::check_file_offset attempts to validate the backing object by checking, among other things, its size to confirm the mapping will fit on it. This check is implemented by using std::fs::metadata to obtain the file size, but this approach doesn't work when the backing object is a block or char device, as the returned value is always zero. In this commit, we replace the use of std::fs:metadata with std::io::Seek::seek(SeekEnd(0)), which works fine even with special files such as block and char devices. As a reference, this is the method that QEMU uses for the same goal. Fixes: rust-vmm#195 Signed-off-by: Sergio Lopez <[email protected]>
Signed-off-by: Jonathan Woollett-Light <[email protected]>
Add ppc64le (powerpc64) and s390x to the list of arches that support AtomicI64 and AtomicU64. Signed-off-by: Sergio Lopez <[email protected]>
Signed-off-by: lsongzhi <[email protected]>
This way the deps that are below 1.0 will only be updated at the patch level, and the ones >= 1.0, at the patch and minor levels. Fixes: rust-vmm#199. Also updated vmm-sys-util to latest version available. Signed-off-by: Laura Loghin <[email protected]>
Fixes: rust-vmm#201. Signed-off-by: Laura Loghin <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `99fe2eb` to `258161e`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@99fe2eb...258161e) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `258161e` to `5f36cc9`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@258161e...5f36cc9) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Obei Sideg <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `5f36cc9` to `7af2e16`. - [Release notes](https://github.com/rust-vmm/rust-vmm-ci/releases) - [Commits](rust-vmm/rust-vmm-ci@5f36cc9...7af2e16) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
All safety comments must start with "SAFETY:". We are not adding safety comments in tests and in the benchmarks. Signed-off-by: Andreea Florescu <[email protected]>
Fixes: rust-vmm#208 Signed-off-by: Andreea Florescu <[email protected]>
Details in the changelog file. Signed-off-by: Andreea Florescu <[email protected]>
The tests module in mmap/windows.rs assumed that `crate::bitmap` was always available, but it is not. Hide the imports / usage of that crate behind appropriate cfgs. Fixes: 4757a4a ("do not backdoor-enable backend-bitmap in unittests") Signed-off-by: Patrick Roy <[email protected]>
Otherwise the crate does not compile on windows with --no-default-features --features backend-bitmap (not a particularly useful combination of features, since it gives bitmaps without mmap backends, which would usually drag in the winapi crate, but it should still work). Signed-off-by: Patrick Roy <[email protected]>
the rawfd and xen features do not work on windows (and don't even compile), so explicitly give a compile_error!() in this case. Signed-off-by: Patrick Roy <[email protected]>
As discussed on PR rust-vmm#349, adding myself to the CODEOWNERS file to help with reviews. Signed-off-by: Hanna Czenczek <[email protected]>
Signed-off-by: Patrick Roy <[email protected]>
Signed-off-by: Patrick Roy <[email protected]>
the `doc_auto_cfg` feature has been merged into `doc_cfg` [1], and this is breaking building vm-memory documentation on docs.rs [2]. [1]: rust-lang/rust#138907 [2]: https://docs.rs/crate/vm-memory/0.17.0/builds/2564760 Signed-off-by: Patrick Roy <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `58dab8b` to `3e24835`. - [Commits](rust-vmm/rust-vmm-ci@58dab8b...3e24835) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-version: 3e248353468a9aa0efa844d39eecf24e1dbdc3f3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `3e24835` to `1b48931`. - [Commits](rust-vmm/rust-vmm-ci@3e24835...1b48931) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-version: 1b48931447cdddebfd19c61f1847e82296fe06ba dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
The warning was given by new clippy
warning: hiding a lifetime that's elided elsewhere is confusing
--> src/bytes.rs:117:17
|
117 | fn as_bytes(&mut self) -> VolatileSlice {
| ^^^^^^^^^ ------------- the same lifetime is hidden here
| |
| the lifetime is elided here
|
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
Signed-off-by: Nguyen Dinh Phi <[email protected]>
The existing `GuestMemory` trait is insufficient for representing virtual memory, as it does not allow specifying the required access permissions. Its focus on all guest memory implementations consisting of a relatively small number of regions is also unsuited for paged virtual memory with a potentially very lage set of non-continuous mappings. The new `IoMemory` trait in contrast provides only a small number of methods that keep the implementing type’s internal structure more opaque, and every access needs to be accompanied by the required permissions. Signed-off-by: Hanna Czenczek <[email protected]>
Rust only allows us to give one trait the blanket implementations for `Bytes`. We want `IoMemory` to be our primary external interface becaue it has users specify the access permissions they need, and because we can (and do) provide a blanket `IoMemory` implementation for all `GuestMemory` types. Also, while `IoMemory` (as the more general trait) only has a restricted interface when compared to `GuestMemory`, this interface is enough to implement `Bytes`; notably, accesses to `IoMemory` require specifying the access mode, which is naturally trivial for `Bytes` methods like `read()` or `write()`. Signed-off-by: Hanna Czenczek <[email protected]>
We want a trait like `GuestAddressSpace` for `IoMemory`, but just
duplicating it into an `IoAddressSpace` trait is not so easy:
We could rename the current `GuestAddressSpace` to `IoAddressSpace` and
require `M: IoMemory` (instead of `M: GuestMemory`), and then define
`GuestAddressSpace` as:
```rust
pub trait GuestAddressSpace: IoAddressSpace<M: GuestMemory> {}
impl<AS: IoAddressSpace> GuestAddressSpace for AS
where
AS::M: GuestMemory,
{}
```
But doing just this would break all existing `GuestAddressSpace` users,
as they’d now need to import `IoAddressSpace` to use `memory()`.
(Re-)Adding `GuestAddressSpace::memory()` as
```rust
fn memory(&self) -> <Self as IoAddressSpace>::T {
IoAddressSpace::memory(self)
}
```
also doesn’t (just) work, as it gets the compiler confused which
`memory()` to use (between `GuestAddressSpace` and `IoAddressSpace`), so
the `IoAddressSpace::memory()` method would need to be called
differently. However, I would find that a bit silly, and it would also
then later require changes if the user wants to switch from
`GuestMemory` to `IoMemory`.
Instead just changing the `GuestAddressSpace::M: GuestMemory`
requirement to `M: IoMemory` seems easier:
- All callers that just use the `Bytes` interface remain completely
unchanged,
- It does break users that actually need the `GuestMemory` interface,
but from what I have seen, that is only the case for
`vhost::vhost_kern`. There, we can simply require that
`<AS as GuestAddressSpace>::M: GuestMemory`.
Signed-off-by: Hanna Czenczek <[email protected]>
This simply makes `GuestMemoryAtomic` more general. (However, this change requires the preceding commit that relaxed the `GuestAddressSpace::M` requirement from `GuestMemory` to `IoMemory`.) Signed-off-by: Hanna Czenczek <[email protected]>
The Iommu trait defines an interface for translating virtual addresses into addresses in an underlying address space. It is supposed to do so by internally keeping an instance of the Iotlb type, updating it with mappings whenever necessary (e.g. when actively invalidated or when there’s an access failure) from some internal data source (e.g. for a vhost-user IOMMU, the data comes from the vhost-user front-end by requesting an update). In a later commit, we are going to provide an implementation of `IoMemory` that can use an `Iommu` to provide an I/O virtual address space. Note that while I/O virtual memory in practice will be organized in pages, the vhost-user specification makes no mention of a specific page size or how to obtain it. Therefore, we cannot really assume any page size and have to use plain ranges with byte granularity as mappings instead. Signed-off-by: Hanna Czenczek <[email protected]>
This `IoMemory` type provides an I/O virtual address space by adding an IOMMU translation layer to an underlying `GuestMemory` object. Signed-off-by: Hanna Czenczek <[email protected]>
The vhost-user-backend crate will need to be able to modify all existing memory regions to use the VMM user address instead of the guest physical address once the IOMMU feature is switched on, and vice versa. To do so, it needs to be able to modify regions’ base address. Because `GuestMemoryMmap` stores regions wrapped in an `Arc<_>`, we cannot mutate them after they have been put into the `GuestMemoryMmap` object; and `MmapRegion` itself is by its nature not clonable. So to modify the regions’ base addresses, we need some way to create a new `GuestRegionMmap` referencing the same `MmapRegion` as another one, but with a different base address. We can do that by having `GuestRegionMmap` wrap its `MmapRegion` in an `Arc`, and adding a method to return that `Arc`, and a method to construct a `GuestRegionMmap` object from such a cloned `Arc.` Signed-off-by: Hanna Czenczek <[email protected]>
Without an IOMMU, we have direct access to guest physical addresses (GPAs). In order to track our writes to guest memory (during migration), we log them into dirty bitmaps, and a page's bit index is its GPA divided by the page size. When it comes to vhost-user, however, and we use an IOMMU, we no longer know the GPA, instead we operate on I/O virtual addresses (IOVAs) and VMM user-space addresses (VUAs). Here, the dirty bitmap bit index is the IOVA divided by the page size. `IoMemory` types contain an internal "physical" memory type that (in case of vhost-user) operates on these VUAs (`IoMemory::PhysicalMemory). Any bitmap functionality that this internal type may already have (e.g. `GuestMemoryMmap` does) cannot be used for vhost-user dirty bitmap tracking with an IOMMU because they would use the VUA, but we need to use the IOVA, and this information is not available on that lower layer. Therefore, `IoMemory` itself needs to support bitmaps separately from its inner `PhysicalMemory`, which will be used when the IOMMU is in use. Add an associated `IoMemory::Bitmap` type and add a bitmap object to `IommuMemory`. Ensure that writes to memory dirty that bitmap appropriately: - In `try_access()`, if write access was requested, dirty the handled region of the bitmap after the access is done. - In `get_slice()`, replace the `VolatileSlice`'s bitmap (which comes from the inner `PhysicalMemory`) by the correct slice of our IOVA bitmap before returning it. Signed-off-by: Hanna Czenczek <[email protected]>
This commit also adds the iommu feature to the coverage_config feature list. (I left the aarch64 coverage value unchanged; I cannot find out how to get the current value on my system, and it isn’t include in CI.) Signed-off-by: Hanna Czenczek <[email protected]>
Document in DESIGN.md how I/O virtual memory is handled. Signed-off-by: Hanna Czenczek <[email protected]>
Signed-off-by: Hanna Czenczek <[email protected]>
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `1b48931` to `042b206`. - [Commits](rust-vmm/rust-vmm-ci@1b48931...042b206) --- updated-dependencies: - dependency-name: rust-vmm-ci dependency-version: 042b2063b276a7593aeee24e8ca4f8eef2c8ac7e dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
The purpose of the function is exactly to change a pointer obtained from a mutable VolatileSlice into a reference, and doing so is unsafe. So just shut up clippy. Signed-off-by: Paolo Bonzini <[email protected]>
Walk the talk and use the intended primary interface to vm-memory. Tests cannot do that because they need to peek at the collection of GuestMemroyRegions. Signed-off-by: Paolo Bonzini <[email protected]>
Rename GuestMemory to GuestMemoryBackend, IoMemory to GuestMemory. This allows users of vm-memory to be aware of access permissions for memory, with no change to callers that just use the `Bytes` interface. Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
After the rename, the primary interface to vm-memory is almost completely absent in DESIGN.md. Give it more importance and remove some details of GuestMemoryBackend that are now obsolete. Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
25 participants
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.
As discusses in rust-vmm#327, make the main interface to vm-memory the one that includes permissions and lets fewer aspects of the implementation through.
Based on rust-vmm#361.