Skip to content

Commit 64bcad5

Browse files
XanClicbonzini
authored andcommitted
Bytes: Do not use to_region_addr()
When we switch to a (potentially) virtual memory model, we want to compact the interface, especially removing references to memory regions because virtual memory is not just split into regions, but pages first. `to_region_addr()` will no longer work then, as it does not communicate to the caller for how many bytes that mapping is valid. (Currently, it is generally valid until the end of the region, but this will not be the case with virtual memory.) So instead, get a `VolatileSlice` via `get_slices()` and do the atomic access on it. (Note: We cannot really have a test case for this, as right now, memory fragmentation will only happen exactly at memory region boundaries. In this case, `region.load()` and `region.store()` would have already returned errors. This change is necessary for when page boundaries result in different mappings within a single region, but because we don’t have IOMMU support yet, this can’t be tested.) Signed-off-by: Hanna Czenczek <[email protected]>
1 parent 1d3c09b commit 64bcad5

File tree

2 files changed

+16
-8
lines changed

2 files changed

+16
-8
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
and `GuestRegionMmap::from_range` to be separate from the error type returned by `GuestRegionCollection` functions.
2424
Change return type of `GuestRegionMmap::new` from `Result` to `Option`.
2525
- \[#324](https:////github.com/rust-vmm/vm-memory/pull/324)\] `GuestMemoryRegion::bitmap()` now returns a `BitmapSlice`. Accessing the full bitmap is now possible only if the type of the memory region is know, for example with `MmapRegion::bitmap()`.
26+
- \[[#339](https://github.com/rust-vmm/vm-memory/pull/339)\] Implement `Bytes::load()` and `Bytes::store()` with `get_slices()` instead of `to_region_addr()`
2627

2728
### Removed
2829

src/guest_memory.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ use std::convert::From;
4545
use std::fs::File;
4646
use std::io;
4747
use std::iter::FusedIterator;
48+
use std::mem::size_of;
4849
use std::ops::{BitAnd, BitOr, Deref};
4950
use std::rc::Rc;
5051
use std::sync::atomic::Ordering;
@@ -685,17 +686,23 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
685686
}
686687

687688
fn store<O: AtomicAccess>(&self, val: O, addr: GuestAddress, order: Ordering) -> Result<()> {
688-
// `find_region` should really do what `to_region_addr` is doing right now, except
689-
// it should keep returning a `Result`.
690-
self.to_region_addr(addr)
691-
.ok_or(Error::InvalidGuestAddress(addr))
692-
.and_then(|(region, region_addr)| region.store(val, region_addr, order))
689+
// No need to check past the first iterator item: It either has the size of `O`, then there
690+
// can be no further items; or it does not, and then `VolatileSlice::store()` will fail.
691+
self.get_slices(addr, size_of::<O>())
692+
.next()
693+
.unwrap()? // count > 0 never produces an empty iterator
694+
.store(val, 0, order)
695+
.map_err(Into::into)
693696
}
694697

695698
fn load<O: AtomicAccess>(&self, addr: GuestAddress, order: Ordering) -> Result<O> {
696-
self.to_region_addr(addr)
697-
.ok_or(Error::InvalidGuestAddress(addr))
698-
.and_then(|(region, region_addr)| region.load(region_addr, order))
699+
// No need to check past the first iterator item: It either has the size of `O`, then there
700+
// can be no further items; or it does not, and then `VolatileSlice::store()` will fail.
701+
self.get_slices(addr, size_of::<O>())
702+
.next()
703+
.unwrap()? // count > 0 never produces an empty iterator
704+
.load(0, order)
705+
.map_err(Into::into)
699706
}
700707
}
701708

0 commit comments

Comments
 (0)