volatile: revert changes introduced by #95

#95 fixed the problem of torn reads/writes caused by the implementation
of read/write_obj essentially leveraging the environment specific
memcpy, and the implicit assumption that read/write_obj perform atomic
accesses up to a certain size at aligned addresses. Meanwhile, we added
the load/store operations to Bytes which provide explicit atomic access
semantics.

Now there are three possible types of memory access methods:
1) atomic access: `VolatileSlice`::load/store() for integer atomic data
   types
2) volatile access: {`VolatileRef`|`VolatileArrayRef`}::{load|store()|
   copy_from<T>|copy_from<T>} when size_of::<T>() > 1
3) normal access: all other byte stream oriented memory accesses

Callers need to be care to choose the access method, in preferrence for
both safety and high performance.

Signed-off-by: Liu Jiang <[email protected]>

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 85.6,
+  "coverage_score": 85.7,
   "exclude_path": "mmap_windows.rs",
   "crate_features": "backend-mmap,backend-atomic"
 }

src/bytes.rs

@@ -149,11 +149,13 @@ byte_valued_type!(u8);
 byte_valued_type!(u16);
 byte_valued_type!(u32);
 byte_valued_type!(u64);
+byte_valued_type!(u128);
 byte_valued_type!(usize);
 byte_valued_type!(i8);
 byte_valued_type!(i16);
 byte_valued_type!(i32);
 byte_valued_type!(i64);
+byte_valued_type!(i128);
 byte_valued_type!(isize);
 
 /// A trait used to identify types which can be accessed atomically by proxy.
@@ -193,6 +195,10 @@ impl_atomic_access!(usize, std::sync::atomic::AtomicUsize);
 
 /// A container to host a range of bytes and access its content.
 ///
+/// This is a byte stream oriented trait. Most data accessors work in byte stream mode logically
+/// and do not guarantee atomicity for data access bigger than a byte. The only exceptions are
+/// `load()` and `store()`, which accesses naturally aligned data in atomic mode.
+///
 /// Candidates which may implement this trait include:
 /// - anonymous memory areas
 /// - mmapped memory areas

src/guest_memory.rs

@@ -403,6 +403,12 @@ impl<M: GuestMemory> GuestAddressSpace for Arc<M> {
 /// The task of the `GuestMemory` trait are:
 /// - map a request address to a `GuestMemoryRegion` object and relay the request to it.
 /// - handle cases where an access request spanning two or more `GuestMemoryRegion` objects.
+///
+/// The `GuestMemory` is a byte stream oriented trait. Most data accessors work in byte stream mode
+/// logically and do not guarantee atomicity for data access bigger than a byte. The only
+/// exceptions are `load()` and `store()`, which accesses naturally aligned data in atomic mode.
+/// Please use `{VolatileRef|VolatileArrayRef}::{load|store()|copy_from<T>|copy_from<T>}` for
+/// volatile accesses.
 pub trait GuestMemory {
     /// Type of objects hosted by the address space.
     type R: GuestMemoryRegion;
@@ -894,8 +900,6 @@ mod tests {
     use crate::{GuestAddress, GuestMemoryMmap};
     #[cfg(feature = "backend-mmap")]
     use std::io::Cursor;
-    #[cfg(feature = "backend-mmap")]
-    use std::time::{Duration, Instant};
 
     use vmm_sys_util::tempfile::TempFile;
 
@@ -936,6 +940,10 @@ mod tests {
         );
     }
 
+    /*
+    #[cfg(feature = "backend-mmap")]
+    use std::time::{Duration, Instant};
+
     // Runs the provided closure in a loop, until at least `duration` time units have elapsed.
     #[cfg(feature = "backend-mmap")]
     fn loop_timed<F>(duration: Duration, mut f: F)
@@ -955,6 +963,7 @@ mod tests {
             }
         }
     }
+     */
 
     // Helper method for the following test. It spawns a writer and a reader thread, which
     // simultaneously try to access an object that is placed at the junction of two memory regions.
@@ -973,7 +982,6 @@ mod tests {
             + PartialEq,
     {
         use std::mem;
-        use std::thread;
 
         // A dummy type that's always going to have the same alignment as the first member,
         // and then adds some bytes at the end.
@@ -1002,12 +1010,10 @@ mod tests {
         ])
         .unwrap();
 
-        // Need to clone this and move it into the new thread we create.
-        let mem2 = mem.clone();
         // Just some bytes.
         let some_bytes = [1u8, 2, 4, 16, 32, 64, 128];
 
-        let mut data = Data {
+        let data = Data {
             val: T::from(0u8),
             some_bytes,
         };
@@ -1017,6 +1023,12 @@ mod tests {
         let read_data = mem.read_obj::<Data<T>>(data_start).unwrap();
         assert_eq!(read_data, data);
 
+        // The GuestMemory/Bytes doesn't guarantee atomicity any, unless load()/store() are used.
+        /*
+        use std::thread;
+
+        // Need to clone this and move it into the new thread we create.
+        let mem2 = mem.clone();
         let t = thread::spawn(move || {
             let mut count: u64 = 0;
 
@@ -1048,6 +1060,7 @@ mod tests {
         });
 
         t.join().unwrap()
+         */
     }
 
     #[cfg(feature = "backend-mmap")]