Switch to IoMemory as the primary memory type

Rust only allows us to give one trait the blanket implementations for
`Bytes` and `GuestAddressSpace`.

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.

Therefore, replace requirements of `GuestMemory` by `IoMemory` instead.

Signed-off-by: Hanna Czenczek <[email protected]>

Author: XanClic

src/atomic.rs

@@ -2,7 +2,7 @@
 // Copyright (C) 2020 Red Hat, Inc. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-//! A wrapper over an `ArcSwap<GuestMemory>` struct to support RCU-style mutability.
+//! A wrapper over an `ArcSwap<IoMemory>` struct to support RCU-style mutability.
 //!
 //! With the `backend-atomic` feature enabled, simply replacing `GuestMemoryMmap`
 //! with `GuestMemoryAtomic<GuestMemoryMmap>` will enable support for mutable memory maps.
@@ -15,17 +15,17 @@ use arc_swap::{ArcSwap, Guard};
 use std::ops::Deref;
 use std::sync::{Arc, LockResult, Mutex, MutexGuard, PoisonError};
 
-use crate::{GuestAddressSpace, GuestMemory};
+use crate::{GuestAddressSpace, IoMemory};
 
 /// A fast implementation of a mutable collection of memory regions.
 ///
 /// This implementation uses `ArcSwap` to provide RCU-like snapshotting of the memory map:
-/// every update of the memory map creates a completely new `GuestMemory` object, and
+/// every update of the memory map creates a completely new `IoMemory` object, and
 /// readers will not be blocked because the copies they retrieved will be collected once
 /// no one can access them anymore.  Under the assumption that updates to the memory map
 /// are rare, this allows a very efficient implementation of the `memory()` method.
 #[derive(Clone, Debug)]
-pub struct GuestMemoryAtomic<M: GuestMemory> {
+pub struct GuestMemoryAtomic<M: IoMemory> {
     // GuestAddressSpace<M>, which we want to implement, is basically a drop-in
     // replacement for &M.  Therefore, we need to pass to devices the `GuestMemoryAtomic`
     // rather than a reference to it.  To obtain this effect we wrap the actual fields
@@ -34,9 +34,9 @@ pub struct GuestMemoryAtomic<M: GuestMemory> {
     inner: Arc<(ArcSwap<M>, Mutex<()>)>,
 }
 
-impl<M: GuestMemory> From<Arc<M>> for GuestMemoryAtomic<M> {
+impl<M: IoMemory> From<Arc<M>> for GuestMemoryAtomic<M> {
     /// create a new `GuestMemoryAtomic` object whose initial contents come from
-    /// the `map` reference counted `GuestMemory`.
+    /// the `map` reference counted `IoMemory`.
     fn from(map: Arc<M>) -> Self {
         let inner = (ArcSwap::new(map), Mutex::new(()));
         GuestMemoryAtomic {
@@ -45,9 +45,9 @@ impl<M: GuestMemory> From<Arc<M>> for GuestMemoryAtomic<M> {
     }
 }
 
-impl<M: GuestMemory> GuestMemoryAtomic<M> {
+impl<M: IoMemory> GuestMemoryAtomic<M> {
     /// create a new `GuestMemoryAtomic` object whose initial contents come from
-    /// the `map` `GuestMemory`.
+    /// the `map` `IoMemory`.
     pub fn new(map: M) -> Self {
         Arc::new(map).into()
     }
@@ -75,7 +75,7 @@ impl<M: GuestMemory> GuestMemoryAtomic<M> {
     }
 }
 
-impl<M: GuestMemory> GuestAddressSpace for GuestMemoryAtomic<M> {
+impl<M: IoMemory> GuestAddressSpace for GuestMemoryAtomic<M> {
     type T = GuestMemoryLoadGuard<M>;
     type M = M;
 
@@ -86,14 +86,14 @@ impl<M: GuestMemory> GuestAddressSpace for GuestMemoryAtomic<M> {
 
 /// A guard that provides temporary access to a `GuestMemoryAtomic`.  This
 /// object is returned from the `memory()` method.  It dereference to
-/// a snapshot of the `GuestMemory`, so it can be used transparently to
+/// a snapshot of the `IoMemory`, so it can be used transparently to
 /// access memory.
 #[derive(Debug)]
-pub struct GuestMemoryLoadGuard<M: GuestMemory> {
+pub struct GuestMemoryLoadGuard<M: IoMemory> {
     guard: Guard<Arc<M>>,
 }
 
-impl<M: GuestMemory> GuestMemoryLoadGuard<M> {
+impl<M: IoMemory> GuestMemoryLoadGuard<M> {
     /// Make a clone of the held pointer and returns it.  This is more
     /// expensive than just using the snapshot, but it allows to hold on
     /// to the snapshot outside the scope of the guard.  It also allows
@@ -104,15 +104,15 @@ impl<M: GuestMemory> GuestMemoryLoadGuard<M> {
     }
 }
 
-impl<M: GuestMemory> Clone for GuestMemoryLoadGuard<M> {
+impl<M: IoMemory> Clone for GuestMemoryLoadGuard<M> {
     fn clone(&self) -> Self {
         GuestMemoryLoadGuard {
             guard: Guard::from_inner(Arc::clone(&*self.guard)),
         }
     }
 }
 
-impl<M: GuestMemory> Deref for GuestMemoryLoadGuard<M> {
+impl<M: IoMemory> Deref for GuestMemoryLoadGuard<M> {
     type Target = M;
 
     fn deref(&self) -> &Self::Target {
@@ -125,12 +125,12 @@ impl<M: GuestMemory> Deref for GuestMemoryLoadGuard<M> {
 /// possibly after updating the memory map represented by the
 /// `GuestMemoryAtomic` that created the guard.
 #[derive(Debug)]
-pub struct GuestMemoryExclusiveGuard<'a, M: GuestMemory> {
+pub struct GuestMemoryExclusiveGuard<'a, M: IoMemory> {
     parent: &'a GuestMemoryAtomic<M>,
     _guard: MutexGuard<'a, ()>,
 }
 
-impl<M: GuestMemory> GuestMemoryExclusiveGuard<'_, M> {
+impl<M: IoMemory> GuestMemoryExclusiveGuard<'_, M> {
     /// Replace the memory map in the `GuestMemoryAtomic` that created the guard
     /// with the new memory map, `map`.  The lock is then dropped since this
     /// method consumes the guard.

src/guest_memory.rs

@@ -55,6 +55,7 @@ use crate::bitmap::{Bitmap, BS, MS};
 use crate::bytes::{AtomicAccess, Bytes};
 use crate::io::{ReadVolatile, WriteVolatile};
 use crate::volatile_memory::{self, VolatileSlice};
+use crate::{IoMemory, Permissions};
 
 /// Errors associated with handling guest memory accesses.
 #[allow(missing_docs)]
@@ -354,7 +355,7 @@ pub trait GuestMemoryRegion: Bytes<MemoryRegionAddress, E = Error> {
 /// ```
 pub trait GuestAddressSpace {
     /// The type that will be used to access guest memory.
-    type M: GuestMemory;
+    type M: IoMemory;
 
     /// A type that provides access to the memory.
     type T: Clone + Deref<Target = Self::M>;
@@ -365,7 +366,7 @@ pub trait GuestAddressSpace {
     fn memory(&self) -> Self::T;
 }
 
-impl<M: GuestMemory> GuestAddressSpace for &M {
+impl<M: IoMemory> GuestAddressSpace for &M {
     type M = M;
     type T = Self;
 
@@ -374,7 +375,7 @@ impl<M: GuestMemory> GuestAddressSpace for &M {
     }
 }
 
-impl<M: GuestMemory> GuestAddressSpace for Rc<M> {
+impl<M: IoMemory> GuestAddressSpace for Rc<M> {
     type M = M;
     type T = Self;
 
@@ -383,7 +384,7 @@ impl<M: GuestMemory> GuestAddressSpace for Rc<M> {
     }
 }
 
-impl<M: GuestMemory> GuestAddressSpace for Arc<M> {
+impl<M: IoMemory> GuestAddressSpace for Arc<M> {
     type M = M;
     type T = Self;
 
@@ -585,13 +586,14 @@ pub trait GuestMemory {
     }
 }
 
-impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
+impl<T: IoMemory + ?Sized> Bytes<GuestAddress> for T {
     type E = Error;
 
     fn write(&self, buf: &[u8], addr: GuestAddress) -> Result<usize> {
         self.try_access(
             buf.len(),
             addr,
+            Permissions::Write,
             |offset, count, caddr, region| -> Result<usize> {
                 region.write(&buf[offset..(offset + count)], caddr)
             },
@@ -602,6 +604,7 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
         self.try_access(
             buf.len(),
             addr,
+            Permissions::Read,
             |offset, count, caddr, region| -> Result<usize> {
                 region.read(&mut buf[offset..(offset + count)], caddr)
             },
@@ -669,9 +672,12 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
     where
         F: ReadVolatile,
     {
-        self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
-            region.read_volatile_from(caddr, src, len)
-        })
+        self.try_access(
+            count,
+            addr,
+            Permissions::Write,
+            |_, len, caddr, region| -> Result<usize> { region.read_volatile_from(caddr, src, len) },
+        )
     }
 
     fn read_exact_volatile_from<F>(
@@ -697,11 +703,16 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
     where
         F: WriteVolatile,
     {
-        self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
-            // For a non-RAM region, reading could have side effects, so we
-            // must use write_all().
-            region.write_all_volatile_to(caddr, dst, len).map(|()| len)
-        })
+        self.try_access(
+            count,
+            addr,
+            Permissions::Read,
+            |_, len, caddr, region| -> Result<usize> {
+                // For a non-RAM region, reading could have side effects, so we
+                // must use write_all().
+                region.write_all_volatile_to(caddr, dst, len).map(|()| len)
+            },
+        )
     }
 
     fn write_all_volatile_to<F>(&self, addr: GuestAddress, dst: &mut F, count: usize) -> Result<()>
@@ -724,6 +735,7 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
         let completed = self.try_access(
             expected,
             addr,
+            Permissions::Write,
             |offset, len, region_addr, region| -> Result<usize> {
                 assert_eq!(offset, 0);
                 if len < expected {
@@ -753,6 +765,7 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
         let completed = self.try_access(
             expected,
             addr,
+            Permissions::Read,
             |offset, len, region_addr, region| -> Result<usize> {
                 assert_eq!(offset, 0);
                 if len < expected {