zerocopy::FromBytes does AlwaysInhabited better

lib/propolis/src/hw/nvme/bits.rs

@@ -5,11 +5,12 @@
 #![allow(dead_code)]
 
 use bitstruct::bitstruct;
+use zerocopy::{FromBytes, FromZeroes};
 
 /// A Submission Queue Entry as represented in memory.
 ///
 /// See NVMe 1.0e Section 4.2 Submission Queue Entry - Command Format
-#[derive(Debug, Default, Copy, Clone)]
+#[derive(Debug, Default, Copy, Clone, FromBytes, FromZeroes)]
 #[repr(C, packed(1))]
 pub struct SubmissionQueueEntry {
     /// Command Dword 0 (CDW0)
@@ -86,9 +87,6 @@ pub struct SubmissionQueueEntry {
     pub cdw15: u32,
 }
 
-/// Safety: all fields of SubmissionQueueEntry are valid for all bit patterns.
-unsafe impl crate::vmm::AlwaysInhabited for SubmissionQueueEntry {}
-
 impl SubmissionQueueEntry {
     /// Returns the Identifier (CID) of this Submission Queue Entry.
     ///

lib/propolis/src/hw/qemu/fwcfg.rs

@@ -815,7 +815,7 @@ mod bits {
     use zerocopy::byteorder::big_endian::{
         U16 as BE16, U32 as BE32, U64 as BE64,
     };
-    use zerocopy::AsBytes;
+    use zerocopy::{AsBytes, FromBytes, FromZeroes};
 
     pub const FW_CFG_IOP_SELECTOR: u16 = 0x0510;
     pub const FW_CFG_IOP_DATA: u16 = 0x0511;
@@ -867,18 +867,13 @@ mod bits {
         }
     }
 
-    #[derive(AsBytes, Default, Copy, Clone, Debug)]
+    #[derive(AsBytes, Default, Copy, Clone, Debug, FromBytes, FromZeroes)]
     #[repr(C)]
     pub struct FwCfgDmaAccess {
         pub ctrl: BE32,
         pub len: BE32,
         pub addr: BE64,
     }
-
-    /// Safety: all fields of FwCfgDmaAccess are valid for all bit patterns.
-    ///
-    /// They're just big-endian instead of little-endian.
-    unsafe impl crate::vmm::AlwaysInhabited for FwCfgDmaAccess {}
 }
 
 #[cfg(test)]

lib/propolis/src/hw/virtio/queue.rs

@@ -17,16 +17,16 @@ use crate::common::*;
 use crate::migrate::MigrateStateError;
 use crate::vmm::MemCtx;
 
+use zerocopy::{FromBytes, FromZeroes};
+
 #[repr(C)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, FromBytes, FromZeroes)]
 struct VqdDesc {
     addr: u64,
     len: u32,
     flags: u16,
     next: u16,
 }
-/// Safety: all fields of VqdDesc are valid for all bit patterns
-unsafe impl crate::vmm::AlwaysInhabited for VqdDesc {}
 #[repr(C)]
 #[derive(Copy, Clone, Debug)]
 struct VqdUsed {

lib/propolis/src/vmm/mem.rs

@@ -19,38 +19,7 @@ use crate::common::{GuestAddr, GuestRegion};
 use crate::util::aspace::ASpace;
 use crate::vmm::VmmHdl;
 
-/// Marker trait declaring that a type is valid for all bit patterns.  This is
-/// one of the necessary requirements to read a type from guest memory.
-///
-/// Notably a type like
-/// ```rust
-/// #[repr(u8)]
-/// enum Foo {
-///     A,
-///     B,
-///     C,
-///     Other(u8)
-/// }
-/// ```
-///
-/// *can not* impl `AlwaysInhabited`: the discriminant may only be one of four
-/// bit pattens.
-///
-/// TODO: it'd be nice to have a proc macro to derive this on structs comprised
-/// entirely of `AlwaysInhabited` types..
-pub unsafe trait AlwaysInhabited {}
-
-/// All bit patterns are valid `u8`.
-unsafe impl AlwaysInhabited for u8 {}
-
-/// All bit patterns are valid `u16`.
-unsafe impl AlwaysInhabited for u16 {}
-
-/// All bit patterns are valid `u64`.
-unsafe impl AlwaysInhabited for u64 {}
-
-/// All bit patterns are valid `u8`, so they are valid `MaybeUninit<u8>` too.
-unsafe impl AlwaysInhabited for MaybeUninit<u8> {}
+use zerocopy::FromBytes;
 
 bitflags! {
     /// Bitflags representing memory protections.
@@ -513,7 +482,7 @@ impl<'a> SubMapping<'a> {
     }
 
     /// Reads a `T` object from the mapping.
-    pub fn read<T: Copy + AlwaysInhabited>(&self) -> Result<T> {
+    pub fn read<T: Copy + FromBytes>(&self) -> Result<T> {
         self.check_read_access()?;
         let typed = self.ptr.as_ptr() as *const T;
         if self.len < std::mem::size_of::<T>() {
@@ -523,12 +492,14 @@ impl<'a> SubMapping<'a> {
         // Safety:
         // - typed must be valid for reads: `check_read_access()` succeeded
         // - typed must point to a properly initialized value of T: always true
-        //     because we require `T: AlwaysInhabited`
+        //     because we require `T: FromBytes`. `zerocopy::FromBytes` happens
+        //     to have the same concerns as us - that T is valid for all bit
+        //     patterns.
         Ok(unsafe { typed.read_unaligned() })
     }
 
     /// Read `values` from the mapping.
-    pub fn read_many<T: Copy + AlwaysInhabited>(
+    pub fn read_many<T: Copy + FromBytes>(
         &self,
         values: &mut [T],
     ) -> Result<()> {
@@ -546,7 +517,7 @@ impl<'a> SubMapping<'a> {
         // will appease those alignment concerns
         let src = self.ptr.as_ptr() as *const u8;
         // We know reinterpreting `*mut T` as `*mut u8` and writing to it cannot
-        // result in invalid `T` because `T: AlwaysInhabited`
+        // result in invalid `T` because `T: FromBytes`
         let dst = values.as_mut_ptr() as *mut u8;
 
         // Safety
@@ -827,10 +798,7 @@ pub struct MemCtx {
 }
 impl MemCtx {
     /// Reads a generic value from a specified guest address.
-    pub fn read<T: Copy + AlwaysInhabited>(
-        &self,
-        addr: GuestAddr,
-    ) -> Option<T> {
+    pub fn read<T: Copy + FromBytes>(&self, addr: GuestAddr) -> Option<T> {
         if let Some(mapping) =
             self.region_covered(addr, size_of::<T>(), Prot::READ)
         {
@@ -870,7 +838,7 @@ impl MemCtx {
     }
 
     /// Reads multiple objects from a guest address.
-    pub fn read_many<T: Copy + AlwaysInhabited>(
+    pub fn read_many<T: Copy + FromBytes>(
         &self,
         base: GuestAddr,
         count: usize,
@@ -1057,7 +1025,7 @@ pub struct MemMany<'a, T: Copy> {
     pos: usize,
     phantom: PhantomData<T>,
 }
-impl<'a, T: Copy + AlwaysInhabited> MemMany<'a, T> {
+impl<'a, T: Copy + FromBytes> MemMany<'a, T> {
     /// Gets the object at position `pos` within the memory region.
     ///
     /// Returns [`Option::None`] if out of range.
@@ -1070,7 +1038,7 @@ impl<'a, T: Copy + AlwaysInhabited> MemMany<'a, T> {
         }
     }
 }
-impl<'a, T: Copy + AlwaysInhabited> Iterator for MemMany<'a, T> {
+impl<'a, T: Copy + FromBytes> Iterator for MemMany<'a, T> {
     type Item = T;
 
     fn next(&mut self) -> Option<Self::Item> {